2007-03-26 22:39:25

by mabbas

Subject: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Add basic support for IEEE 802.11n discovery and association.

This patch adds support to discover IEEE 802.11n AP and enable
association to 802.11n Network. It parses beacon to discover 802.11n
IE and include HT capability information element in Association Request
Frame.
It also call low level driver with the HT capability available during
association.

Signed-off-by: Mohamed Abbas <[email protected]>

diff -Nupr wireless-dev/include/net/mac80211.h
wireless-dev-new/include/net/mac80211.h
--- wireless-dev/include/net/mac80211.h 2007-03-27 00:36:28.000000000
-0700
+++ wireless-dev-new/include/net/mac80211.h 2007-03-27
00:59:40.000000000 -0700
@@ -526,6 +526,9 @@ struct ieee80211_hw {
* per-packet RC4 key with each TX frame when doing hwcrypto */
#define IEEE80211_HW_TKIP_REQ_PHASE2_KEY (1<<14)

+ /* The device capable of supporting 11n */
+#define IEEE80211_HW_SUPPORT_HT_MODE (1<<15)
+
u32 flags; /* hardware flags defined above */

/* Set to the size of a needed device specific skb headroom for TX
skbs. */
@@ -720,6 +723,17 @@ struct ieee80211_ops {
/* Get the current TSF timer value from firmware/hardware. Currently,
* this is only used for IBSS mode debugging and, as such, is not a
* required function. */
+
+ /* Configure ht parameters
+ */
+ int (*conf_ht)(struct ieee80211_hw *hw,
+ struct ieee80211_ht_capability *ht_cap_param,
+ struct ieee80211_ht_additional_info *ht_extra_param);
+
+ /* Get ht capabilities from the device */
+ int (*get_ht_capab)(struct ieee80211_hw *hw,
+ struct ieee80211_ht_capability *ht_cap_param);
+
u64 (*get_tsf)(struct ieee80211_hw *hw);

/* Reset the TSF timer and allow firmware/hardware to synchronize with
diff -Nupr wireless-dev/net/mac80211/ieee80211_i.h
wireless-dev-new/net/mac80211/ieee80211_i.h
--- wireless-dev/net/mac80211/ieee80211_i.h 2007-03-27
00:36:28.000000000 -0700
+++ wireless-dev-new/net/mac80211/ieee80211_i.h 2007-03-27
00:59:40.000000000 -0700
@@ -91,6 +91,8 @@ struct ieee80211_sta_bss {
size_t rsn_ie_len;
u8 *wmm_ie;
size_t wmm_ie_len;
+ u8 *ht_ie;
+ size_t ht_ie_len;
#define IEEE80211_MAX_SUPP_RATES 32
u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
size_t supp_rates_len;
@@ -269,6 +271,7 @@ struct ieee80211_if_sta {
unsigned int create_ibss:1;
unsigned int mixed_cell:1;
unsigned int wmm_enabled:1;
+ unsigned int ht_enabled:1;
unsigned int auto_ssid_sel:1;
unsigned int auto_bssid_sel:1;
unsigned int auto_channel_sel:1;
diff -Nupr wireless-dev/net/mac80211/ieee80211_iface.c
wireless-dev-new/net/mac80211/ieee80211_iface.c
--- wireless-dev/net/mac80211/ieee80211_iface.c 2007-03-27
00:36:28.000000000 -0700
+++ wireless-dev-new/net/mac80211/ieee80211_iface.c 2007-03-27
00:59:40.000000000 -0700
@@ -191,6 +191,7 @@ void ieee80211_if_set_type(struct net_de
IEEE80211_AUTH_ALG_SHARED_KEY;
ifsta->create_ibss = 1;
ifsta->wmm_enabled = 1;
+ ifsta->ht_enabled = 1;
ifsta->auto_channel_sel = 1;
ifsta->auto_bssid_sel = 1;

diff -Nupr wireless-dev/net/mac80211/ieee80211_sta.c
wireless-dev-new/net/mac80211/ieee80211_sta.c
--- wireless-dev/net/mac80211/ieee80211_sta.c 2007-03-27
00:36:28.000000000 -0700
+++ wireless-dev-new/net/mac80211/ieee80211_sta.c 2007-03-27
01:26:06.000000000 -0700
@@ -96,6 +96,10 @@ struct ieee802_11_elems {
u8 rsn_len;
u8 *erp_info;
u8 erp_info_len;
+ u8 *ht_cap_param;
+ u8 ht_cap_param_len;
+ u8 *ht_extra_param;
+ u8 ht_extra_param_len;
u8 *ext_supp_rates;
u8 ext_supp_rates_len;
u8 *wmm_info;
@@ -197,6 +201,14 @@ static ParseRes ieee802_11_parse_elems(u
elems->ext_supp_rates = pos;
elems->ext_supp_rates_len = elen;
break;
+ case WLAN_EID_HT_CAPABILITY:
+ elems->ht_cap_param = pos;
+ elems->ht_cap_param_len = elen;
+ break;
+ case WLAN_EID_HT_EXTRA_INFO:
+ elems->ht_extra_param = pos;
+ elems->ht_extra_param_len = elen;
+ break;
default:
#if 0
printk(KERN_DEBUG "IEEE 802.11 element parse ignored "
@@ -230,7 +242,55 @@ static int ecw2cw(int ecw)
return cw - 1;
}

+/* call low level driver with 11n params as it was recieved
+ from the AP
+*/
+static void ieee80211_sta_ht_params(struct net_device *dev,
+ struct sta_info *sta,
+ struct ieee80211_ht_additional_info *ht_extra_param,
+ struct ieee80211_ht_capability *ht_cap_param)
+{
+ int rc;
+ struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+
+ if (local->ops->conf_ht) {
+ rc = local->ops->conf_ht(local_to_hw(local), ht_cap_param,
+ ht_extra_param);
+
+ if (rc)
+ sta->flags &= ~WLAN_STA_HT;
+ } else
+ sta->flags &= ~WLAN_STA_HT;
+
+ return;
+}
+
+/* Get 11n capabilties from low level driver */
+static void ieee80211_fill_ht_ie(struct net_device *dev,
+ struct ieee80211_ht_capability *ht_capab)
+{
+ int rc;
+ struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+
+ if (!local->ops->get_ht_capab){
+ memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
+ return;
+ }
+
+ rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
+ if (!rc) {
+ memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
+ return;
+ }

+ ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
+ ht_capab->capabilitiesInfo);
+ ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
+ ht_capab->extended_ht_capability_info);
+ ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
+ ht_capab->tx_BF_capability_info);
+}
+
static void ieee80211_sta_wmm_params(struct net_device *dev,
struct ieee80211_if_sta *ifsta,
u8 *wmm_param, size_t wmm_param_len)
@@ -487,6 +547,7 @@ static void ieee80211_send_assoc(struct
u16 capab;
struct ieee80211_sta_bss *bss;
int wmm = 0;
+ int ht_enabled = 0;

skb = dev_alloc_skb(local->hw.extra_tx_headroom +
sizeof(*mgmt) + 200 + ifsta->extra_ie_len +
@@ -509,6 +570,9 @@ static void ieee80211_send_assoc(struct
capab |= WLAN_CAPABILITY_PRIVACY;
if (bss->wmm_ie) {
wmm = 1;
+
+ if (bss->ht_ie)
+ ht_enabled = 1;
}
ieee80211_rx_bss_put(dev, bss);
}
@@ -584,6 +648,15 @@ static void ieee80211_send_assoc(struct
*pos++ = 0;
}

+ /* if low level driver support 11n fill in 11n IE */
+ if (ht_enabled && ifsta->ht_enabled && local->ops->get_ht_capab) {
+ pos = skb_put(skb, sizeof(struct ieee80211_ht_capability)+2);
+ *pos++ = WLAN_EID_HT_CAPABILITY;
+ *pos++ = sizeof(struct ieee80211_ht_capability);
+ ieee80211_fill_ht_ie(dev,
+ (struct ieee80211_ht_capability *)pos);
+ }
+
kfree(ifsta->assocreq_ies);
ifsta->assocreq_ies_len = (skb->data + skb->len) - ies;
ifsta->assocreq_ies = kmalloc(ifsta->assocreq_ies_len, GFP_ATOMIC);
@@ -1215,6 +1288,16 @@ static void ieee80211_rx_mgmt_assoc_resp
}
sta->supp_rates = rates;

+ if (elems.ht_extra_param && elems.ht_cap_param && elems.wmm_param &&
+ ifsta->ht_enabled ) {
+ sta->flags |= WLAN_STA_HT;
+ ieee80211_sta_ht_params(dev, sta,
+ (struct ieee80211_ht_additional_info *)
+ elems.ht_extra_param,
+ (struct ieee80211_ht_capability *)
+ elems.ht_cap_param);
+ }
+
rate_control_rate_init(sta, local);

if (elems.wmm_param && ifsta->wmm_enabled) {
@@ -1310,6 +1393,7 @@ static void ieee80211_rx_bss_free(struct
kfree(bss->wpa_ie);
kfree(bss->rsn_ie);
kfree(bss->wmm_ie);
+ kfree(bss->ht_ie);
kfree(bss);
}

@@ -1556,6 +1640,22 @@ static void ieee80211_rx_bss_info(struct
bss->wmm_ie_len = 0;
}

+ if (elems.ht_cap_param &&
+ (!bss->ht_ie || bss->ht_ie_len != elems.ht_cap_param_len ||
+ memcmp(bss->ht_ie, elems.ht_cap_param, elems.ht_cap_param_len)))
{
+ kfree(bss->ht_ie);
+ bss->ht_ie = kmalloc(elems.ht_cap_param_len + 2, GFP_ATOMIC);
+ if (bss->ht_ie) {
+ memcpy(bss->ht_ie, elems.ht_cap_param - 2,
+ elems.ht_cap_param_len + 2);
+ bss->ht_ie_len = elems.ht_cap_param_len + 2;
+ } else
+ bss->ht_ie_len = 0;
+ } else if (!elems.ht_cap_param && bss->ht_ie) {
+ kfree(bss->ht_ie);
+ bss->ht_ie = NULL;
+ bss->ht_ie_len = 0;
+ }

bss->hw_mode = rx_status->phymode;
bss->channel = channel;
diff -Nupr wireless-dev/net/mac80211/ieee80211_sysfs_sta.c
wireless-dev-new/net/mac80211/ieee80211_sysfs_sta.c
--- wireless-dev/net/mac80211/ieee80211_sysfs_sta.c 2007-03-27
00:36:28.000000000 -0700
+++ wireless-dev-new/net/mac80211/ieee80211_sysfs_sta.c 2007-03-27
00:59:40.000000000 -0700
@@ -79,7 +79,7 @@ STA_ATTR(wep_weak_iv_count, wep_weak_iv_

static ssize_t show_sta_flags(const struct sta_info *sta, char *buf)
{
- return sprintf(buf, "%s%s%s%s%s%s%s%s%s",
+ return sprintf(buf, "%s%s%s%s%s%s%s%s%s%s",
sta->flags & WLAN_STA_AUTH ? "AUTH\n" : "",
sta->flags & WLAN_STA_ASSOC ? "ASSOC\n" : "",
sta->flags & WLAN_STA_PS ? "PS\n" : "",
@@ -89,7 +89,8 @@ static ssize_t show_sta_flags(const stru
sta->flags & WLAN_STA_SHORT_PREAMBLE ?
"SHORT PREAMBLE\n" : "",
sta->flags & WLAN_STA_WME ? "WME\n" : "",
- sta->flags & WLAN_STA_WDS ? "WDS\n" : "");
+ sta->flags & WLAN_STA_WDS ? "WDS\n" : "",
+ sta->flags & WLAN_STA_HT ? "HT\n" : "");
}
__STA_ATTR(flags);

diff -Nupr wireless-dev/net/mac80211/sta_info.h
wireless-dev-new/net/mac80211/sta_info.h
--- wireless-dev/net/mac80211/sta_info.h 2007-03-27 00:36:28.000000000
-0700
+++ wireless-dev-new/net/mac80211/sta_info.h 2007-03-27
00:59:40.000000000 -0700
@@ -26,6 +26,7 @@
*/
#define WLAN_STA_SHORT_PREAMBLE BIT(7)
#define WLAN_STA_WME BIT(9)
+#define WLAN_STA_HT BIT(10)
#define WLAN_STA_WDS BIT(27)

2007-03-28 17:55:42

by mabbas

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Wed, 2007-03-28 at 11:14 +0200, Johannes Berg wrote:
> On Mon, 2007-03-26 at 04:38 -0700, mohamed wrote:
>
> > + /* Get ht capabilities from the device */
> > + int (*get_ht_capab)(struct ieee80211_hw *hw,
> > + struct ieee80211_ht_capability *ht_cap_param);
> > +
>
> Does this really need to return an error? And if so, what happens? You
> currently send a blank HT capabilities IE but I think in that case it
> should be left out completely. But I don't think it will ever return an
> error, it doesn't even need to allocate memory. Just change this to
> void?
>
> johannes
Since we are setting the structure to all 0 before calling the function
I guess it is safe to change the function to return void. I will
resubmit the patch with that fix
Mohamed

2007-03-29 19:46:38

by Luis R. Rodriguez

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On 3/28/07, Johannes Berg <[email protected]> wrote:

> > + /* if low level driver support 11n fill in 11n IE */
> > + if (ht_enabled && ifsta->ht_enabled && local->ops->get_ht_capab) {
> > + pos = skb_put(skb, sizeof(struct ieee80211_ht_capability)+2);
> > + *pos++ = WLAN_EID_HT_CAPABILITY;
> > + *pos++ = sizeof(struct ieee80211_ht_capability);
> > + ieee80211_fill_ht_ie(dev,
> > + (struct ieee80211_ht_capability *)pos);
>
> Now that fill_ht_ie is so short maybe it should just be rolled into this
> code.

True, might as well.

Luis

2007-03-27 17:11:46

by Luis R. Rodriguez

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> On 3/26/07, mohamed <[email protected]> wrote:
>
> > +/* Get 11n capabilties from low level driver */
> > +static void ieee80211_fill_ht_ie(struct net_device *dev,
> > + struct ieee80211_ht_capability *ht_capab)
> > +{
> > + int rc;
> > + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > +
> > + if (!local->ops->get_ht_capab){
> > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > + return;
> > + }
> > +
> > + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> > + if (!rc) {
> > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > + return;
> > + }
> >
> > + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > + ht_capab->capabilitiesInfo);
> > + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > + ht_capab->extended_ht_capability_info);
> > + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > + ht_capab->tx_BF_capability_info);
> > +}
> > +
>
> We should memset to 0 the entire ht_capab to regardless as its coming
> from skb_put() otherwise we'll get random data there if we don't set
> it and we sure as hell don't want to just transmit that :) Also
> ieee80211_send_assoc() is already checking for
> local->ops->get_ht_capab so lets just do a BUG_ON here to capture
> cases where someone else didn't do the proper checking. So how about
> instead something like:
>
> /* Get 11n capabilties from low level driver */
> static void ieee80211_fill_ht_ie(struct net_device *dev,
> struct ieee80211_ht_capability *ht_capab)
> {
> struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> BUG_ON(!local->ops->get_ht_capab);
> memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> if (!local->ops->get_ht_capab(local_to_hw(local), ht_capab))
> return;
> ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> ht_capab->capabilitiesInfo);
> ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> ht_capab->extended_ht_capability_info);
> ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> ht_capab->tx_BF_capability_info);
> }
>

On second thought just memset to 0 on ieee80211_send_assoc() right
after the skb_put and remove that from above.

Luis

2007-03-29 11:07:42

by Johannes Berg

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Mon, 2007-03-26 at 04:38 -0700, mohamed wrote:

> @@ -96,6 +96,10 @@ struct ieee802_11_elems {
> u8 rsn_len;
> u8 *erp_info;
> u8 erp_info_len;
> + u8 *ht_cap_param;
> + u8 ht_cap_param_len;
> + u8 *ht_extra_param;
> + u8 ht_extra_param_len;
> u8 *ext_supp_rates;
> u8 ext_supp_rates_len;
> u8 *wmm_info;

Random note not applicable to your patch: This is quite a large struct,
especially on 64-bit. Maybe we should be thinking about making those
pointers there u16 offsets instead. And we *definitely* should change
the order of these fields, having u8 and u8* alternate just sucks.

> +static void ieee80211_sta_ht_params(struct net_device *dev,
> + struct sta_info *sta,
> + struct ieee80211_ht_additional_info *ht_extra_param,
> + struct ieee80211_ht_capability *ht_cap_param)
> +{
> + int rc;
> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> +
> + if (local->ops->conf_ht) {
> + rc = local->ops->conf_ht(local_to_hw(local), ht_cap_param,
> + ht_extra_param);
> +
> + if (rc)
> + sta->flags &= ~WLAN_STA_HT;
> + } else
> + sta->flags &= ~WLAN_STA_HT;

Shouldn't this also set the STA_HT flag in the case where conf_ht
returns without error?

> +/* Get 11n capabilties from low level driver */
> +static void ieee80211_fill_ht_ie(struct net_device *dev,

[filling in how it looks like after Luis's and my proposed changes]

> + struct ieee80211_ht_capability *ht_capab)
> +{
> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> + BUG_ON(!local->ops->get_ht_capab);
> + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> + local->ops->get_ht_capab(local_to_hw(local), ht_capab);

some blank lines would be good ;)

Maybe we should be setting some fields to default values here that
aren't zero? Haven't checked.

> + /* if low level driver support 11n fill in 11n IE */
> + if (ht_enabled && ifsta->ht_enabled && local->ops->get_ht_capab) {
> + pos = skb_put(skb, sizeof(struct ieee80211_ht_capability)+2);
> + *pos++ = WLAN_EID_HT_CAPABILITY;
> + *pos++ = sizeof(struct ieee80211_ht_capability);
> + ieee80211_fill_ht_ie(dev,
> + (struct ieee80211_ht_capability *)pos);

Now that fill_ht_ie is so short maybe it should just be rolled into this
code.

johannes

Attachments:

2007-03-27 17:05:42

by Luis R. Rodriguez

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On 3/26/07, mohamed <[email protected]> wrote:

> +/* Get 11n capabilties from low level driver */
> +static void ieee80211_fill_ht_ie(struct net_device *dev,
> + struct ieee80211_ht_capability *ht_capab)
> +{
> + int rc;
> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> +
> + if (!local->ops->get_ht_capab){
> + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> + return;
> + }
> +
> + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> + if (!rc) {
> + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> + return;
> + }
>
> + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> + ht_capab->capabilitiesInfo);
> + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> + ht_capab->extended_ht_capability_info);
> + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> + ht_capab->tx_BF_capability_info);
> +}
> +

We should memset to 0 the entire ht_capab to regardless as its coming
from skb_put() otherwise we'll get random data there if we don't set
it and we sure as hell don't want to just transmit that :) Also
ieee80211_send_assoc() is already checking for
local->ops->get_ht_capab so lets just do a BUG_ON here to capture
cases where someone else didn't do the proper checking. So how about
instead something like:

/* Get 11n capabilties from low level driver */
static void ieee80211_fill_ht_ie(struct net_device *dev,
struct ieee80211_ht_capability *ht_capab)
{
struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
BUG_ON(!local->ops->get_ht_capab);
memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
if (!local->ops->get_ht_capab(local_to_hw(local), ht_capab))
return;
ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
ht_capab->capabilitiesInfo);
ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
ht_capab->extended_ht_capability_info);
ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
ht_capab->tx_BF_capability_info);
}

2007-03-30 12:12:21

by Johannes Berg

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Text email is great. But the useless attachment isn't.

>
> Anyhow I still would like to repeat my question unrelated to the
> discussion above since I don't see IE structs defined in headers
> unlike the old wireless stack. (net/ieee80211.h)

I don't understand you. What's wrong with linux/ieee80211.h and
Mohamed's patch 1/5?

johannes

Attachments:

2007-03-28 19:11:15

by Luis R. Rodriguez

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On 3/29/07, mohamed <[email protected]> wrote:
> On Tue, 2007-03-27 at 13:29 -0400, Luis R. Rodriguez wrote:
> > On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> > > On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> > > > On 3/26/07, mohamed <[email protected]> wrote:
> > > >
> > > > > +/* Get 11n capabilties from low level driver */
> > > > > +static void ieee80211_fill_ht_ie(struct net_device *dev,
> > > > > + struct ieee80211_ht_capability *ht_capab)
> > > > > +{
> > > > > + int rc;
> > > > > + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > > > > +
> > > > > + if (!local->ops->get_ht_capab){
> > > > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> > > > > + if (!rc) {
> > > > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > > + return;
> > > > > + }
> > > > >
> > > > > + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > > > > + ht_capab->capabilitiesInfo);
> > > > > + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > > > > + ht_capab->extended_ht_capability_info);
> > > > > + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > > > > + ht_capab->tx_BF_capability_info);
> > > > > +}
> > > > > +
> > > >
> > > > We should memset to 0 the entire ht_capab to regardless as its coming
> > > > from skb_put() otherwise we'll get random data there if we don't set
> > > > it and we sure as hell don't want to just transmit that :)
> I will memset to 0 before using the structure. I will change the patch
> to reflect this.

Thanks

> > Also
> > > > ieee80211_send_assoc() is already checking for
> > > > local->ops->get_ht_capab so lets just do a BUG_ON here to capture
> > > > cases where someone else didn't do the proper checking. So how about
> > > > instead something like:
> make sense.

Thanks

> > > > /* Get 11n capabilties from low level driver */
> > > > static void ieee80211_fill_ht_ie(struct net_device *dev,
> > > > struct ieee80211_ht_capability *ht_capab)
> > > > {
> > > > struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > > > BUG_ON(!local->ops->get_ht_capab);
> > > > memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > if (!local->ops->get_ht_capab(local_to_hw(local), ht_capab))
> > > > return;
> > > > ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > > > ht_capab->capabilitiesInfo);
> > > > ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > > > ht_capab->extended_ht_capability_info);
> > > > ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > > > ht_capab->tx_BF_capability_info);
> > > > }
> > > >
> > >
> > > On second thought just memset to 0 on ieee80211_send_assoc() right
> > > after the skb_put and remove that from above.
> > >
> > > Luis
> > >
> >
> > Last few comments:
> >
> > I. In struct ieee80211_ops you added your hunk after the TSF comment,
> > but before u64 (*get_tsf)(struct ieee80211_hw *hw) was listed. Please
> > move the hunk after get_tsf or before the comment.
> >
> > II. In ieee80211_fill_ht_ie() you do cpu_to_le* on the same fields.. I
> > take it the driver's get_ht_capab() is supposed to fill these. Since
> > you already defined them as little endian in the struct
> > ieee80211_ht_capability, how about just leaving that up to the driver
> > developer to make sure they do this? That's would be 6 lines less of
> > code in ieee80211_fill_ht_ie().

And how about regarding the above comment (II)? We can just expect
driver developers to follow proper endianness. That cleans that
routine a bit more too.

Luis

2007-03-26 23:54:00

by Randy Dunlap

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Mon, 26 Mar 2007 04:38:25 -0700 mohamed wrote:

> Add basic support for IEEE 802.11n discovery and association.
>
> This patch adds support to discover IEEE 802.11n AP and enable
> association to 802.11n Network. It parses beacon to discover 802.11n
> IE and include HT capability information element in Association Request
> Frame.
> It also call low level driver with the HT capability available during
> association.
>
> Signed-off-by: Mohamed Abbas <[email protected]>
>
> diff -Nupr wireless-dev/include/net/mac80211.h
> wireless-dev-new/include/net/mac80211.h
> --- wireless-dev/include/net/mac80211.h 2007-03-27 00:36:28.000000000
> -0700
> +++ wireless-dev-new/include/net/mac80211.h 2007-03-27
> 00:59:40.000000000 -0700

I'm seeing an email/patch problem here. The lines above should be
3 lines only, not 6 that I'm seeing. Maybe it's my mail client,
but it usually works for me, so possibly yours is splitting lines
when it should not.

> @@ -526,6 +526,9 @@ struct ieee80211_hw {
> * per-packet RC4 key with each TX frame when doing hwcrypto */
> #define IEEE80211_HW_TKIP_REQ_PHASE2_KEY (1<<14)
>
> + /* The device capable of supporting 11n */
> +#define IEEE80211_HW_SUPPORT_HT_MODE (1<<15)
> +
> u32 flags; /* hardware flags defined above */
>
> /* Set to the size of a needed device specific skb headroom for TX
> skbs. */
> @@ -720,6 +723,17 @@ struct ieee80211_ops {
> /* Get the current TSF timer value from firmware/hardware. Currently,
> * this is only used for IBSS mode debugging and, as such, is not a
> * required function. */
> +
> + /* Configure ht parameters

Line above ends with space. :(

What is "ht"?

> + */
> + int (*conf_ht)(struct ieee80211_hw *hw,
> + struct ieee80211_ht_capability *ht_cap_param,
> + struct ieee80211_ht_additional_info *ht_extra_param);
> +
> + /* Get ht capabilities from the device */
> + int (*get_ht_capab)(struct ieee80211_hw *hw,
> + struct ieee80211_ht_capability *ht_cap_param);
> +
> u64 (*get_tsf)(struct ieee80211_hw *hw);
>
> /* Reset the TSF timer and allow firmware/hardware to synchronize with
> diff -Nupr wireless-dev/net/mac80211/ieee80211_sta.c
> wireless-dev-new/net/mac80211/ieee80211_sta.c
> --- wireless-dev/net/mac80211/ieee80211_sta.c 2007-03-27
> 00:36:28.000000000 -0700
> +++ wireless-dev-new/net/mac80211/ieee80211_sta.c 2007-03-27
> 01:26:06.000000000 -0700
> @@ -230,7 +242,55 @@ static int ecw2cw(int ecw)
> return cw - 1;
> }
>
> +/* call low level driver with 11n params as it was recieved

received
> + from the AP
> +*/

Line above ends with spaces.

Kernel long comment style is:

/*
* call low-level driver with 11n params as they were received
* from the AP
*/

> +static void ieee80211_sta_ht_params(struct net_device *dev,
> + struct sta_info *sta,
> + struct ieee80211_ht_additional_info *ht_extra_param,
> + struct ieee80211_ht_capability *ht_cap_param)
> +{
> + int rc;
> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> +
> + if (local->ops->conf_ht) {
> + rc = local->ops->conf_ht(local_to_hw(local), ht_cap_param,
> + ht_extra_param);
> +
> + if (rc)
> + sta->flags &= ~WLAN_STA_HT;
> + } else
> + sta->flags &= ~WLAN_STA_HT;
> +
> + return;
> +}
> +
> +/* Get 11n capabilties from low level driver */
> +static void ieee80211_fill_ht_ie(struct net_device *dev,
> + struct ieee80211_ht_capability *ht_capab)
> +{
> + int rc;
> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> +
> + if (!local->ops->get_ht_capab){
> + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> + return;
> + }
> +
> + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> + if (!rc) {
> + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> + return;
> + }
>
> + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> + ht_capab->capabilitiesInfo);
> + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> + ht_capab->extended_ht_capability_info);
> + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> + ht_capab->tx_BF_capability_info);
> +}
> +
> static void ieee80211_sta_wmm_params(struct net_device *dev,
> struct ieee80211_if_sta *ifsta,
> u8 *wmm_param, size_t wmm_param_len)
> @@ -584,6 +648,15 @@ static void ieee80211_send_assoc(struct
> *pos++ = 0;
> }
>
> + /* if low level driver support 11n fill in 11n IE */

supports 11n,

> + if (ht_enabled && ifsta->ht_enabled && local->ops->get_ht_capab) {
> + pos = skb_put(skb, sizeof(struct ieee80211_ht_capability)+2);
> + *pos++ = WLAN_EID_HT_CAPABILITY;
> + *pos++ = sizeof(struct ieee80211_ht_capability);
> + ieee80211_fill_ht_ie(dev,
> + (struct ieee80211_ht_capability *)pos);
> + }
> +
> kfree(ifsta->assocreq_ies);
> ifsta->assocreq_ies_len = (skb->data + skb->len) - ies;
> ifsta->assocreq_ies = kmalloc(ifsta->assocreq_ies_len, GFP_ATOMIC);


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2007-03-28 18:03:16

by mabbas

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Tue, 2007-03-27 at 13:29 -0400, Luis R. Rodriguez wrote:
> On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> > On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> > > On 3/26/07, mohamed <[email protected]> wrote:
> > >
> > > > +/* Get 11n capabilties from low level driver */
> > > > +static void ieee80211_fill_ht_ie(struct net_device *dev,
> > > > + struct ieee80211_ht_capability *ht_capab)
> > > > +{
> > > > + int rc;
> > > > + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > > > +
> > > > + if (!local->ops->get_ht_capab){
> > > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > + return;
> > > > + }
> > > > +
> > > > + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> > > > + if (!rc) {
> > > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > + return;
> > > > + }
> > > >
> > > > + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > > > + ht_capab->capabilitiesInfo);
> > > > + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > > > + ht_capab->extended_ht_capability_info);
> > > > + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > > > + ht_capab->tx_BF_capability_info);
> > > > +}
> > > > +
> > >
> > > We should memset to 0 the entire ht_capab to regardless as its coming
> > > from skb_put() otherwise we'll get random data there if we don't set
> > > it and we sure as hell don't want to just transmit that :)
I will memset to 0 before using the structure. I will change the patch
to reflect this.
> Also
> > > ieee80211_send_assoc() is already checking for
> > > local->ops->get_ht_capab so lets just do a BUG_ON here to capture
> > > cases where someone else didn't do the proper checking. So how about
> > > instead something like:
make sense.
> > >
> > > /* Get 11n capabilties from low level driver */
> > > static void ieee80211_fill_ht_ie(struct net_device *dev,
> > > struct ieee80211_ht_capability *ht_capab)
> > > {
> > > struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > > BUG_ON(!local->ops->get_ht_capab);
> > > memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > if (!local->ops->get_ht_capab(local_to_hw(local), ht_capab))
> > > return;
> > > ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > > ht_capab->capabilitiesInfo);
> > > ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > > ht_capab->extended_ht_capability_info);
> > > ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > > ht_capab->tx_BF_capability_info);
> > > }
> > >
> >
> > On second thought just memset to 0 on ieee80211_send_assoc() right
> > after the skb_put and remove that from above.
> >
> > Luis
> >
>
> Last few comments:
>
> I. In struct ieee80211_ops you added your hunk after the TSF comment,
> but before u64 (*get_tsf)(struct ieee80211_hw *hw) was listed. Please
> move the hunk after get_tsf or before the comment.
>
> II. In ieee80211_fill_ht_ie() you do cpu_to_le* on the same fields.. I
> take it the driver's get_ht_capab() is supposed to fill these. Since
> you already defined them as little endian in the struct
> ieee80211_ht_capability, how about just leaving that up to the driver
> developer to make sure they do this? That's would be 6 lines less of
> code in ieee80211_fill_ht_ie().
>
> Luis

2007-03-27 17:29:16

by Luis R. Rodriguez

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> > On 3/26/07, mohamed <[email protected]> wrote:
> >
> > > +/* Get 11n capabilties from low level driver */
> > > +static void ieee80211_fill_ht_ie(struct net_device *dev,
> > > + struct ieee80211_ht_capability *ht_capab)
> > > +{
> > > + int rc;
> > > + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > > +
> > > + if (!local->ops->get_ht_capab){
> > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > + return;
> > > + }
> > > +
> > > + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> > > + if (!rc) {
> > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > + return;
> > > + }
> > >
> > > + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > > + ht_capab->capabilitiesInfo);
> > > + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > > + ht_capab->extended_ht_capability_info);
> > > + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > > + ht_capab->tx_BF_capability_info);
> > > +}
> > > +
> >
> > We should memset to 0 the entire ht_capab to regardless as its coming
> > from skb_put() otherwise we'll get random data there if we don't set
> > it and we sure as hell don't want to just transmit that :) Also
> > ieee80211_send_assoc() is already checking for
> > local->ops->get_ht_capab so lets just do a BUG_ON here to capture
> > cases where someone else didn't do the proper checking. So how about
> > instead something like:
> >
> > /* Get 11n capabilties from low level driver */
> > static void ieee80211_fill_ht_ie(struct net_device *dev,
> > struct ieee80211_ht_capability *ht_capab)
> > {
> > struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > BUG_ON(!local->ops->get_ht_capab);
> > memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > if (!local->ops->get_ht_capab(local_to_hw(local), ht_capab))
> > return;
> > ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > ht_capab->capabilitiesInfo);
> > ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > ht_capab->extended_ht_capability_info);
> > ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > ht_capab->tx_BF_capability_info);
> > }
> >
>
> On second thought just memset to 0 on ieee80211_send_assoc() right
> after the skb_put and remove that from above.
>
> Luis
>

Last few comments:

I. In struct ieee80211_ops you added your hunk after the TSF comment,
but before u64 (*get_tsf)(struct ieee80211_hw *hw) was listed. Please
move the hunk after get_tsf or before the comment.

II. In ieee80211_fill_ht_ie() you do cpu_to_le* on the same fields.. I
take it the driver's get_ht_capab() is supposed to fill these. Since
you already defined them as little endian in the struct
ieee80211_ht_capability, how about just leaving that up to the driver
developer to make sure they do this? That's would be 6 lines less of
code in ieee80211_fill_ht_ie().

Luis

2007-03-28 19:20:52

by mabbas

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Wed, 2007-03-28 at 15:11 -0400, Luis R. Rodriguez wrote:
> On 3/29/07, mohamed <[email protected]> wrote:
> > On Tue, 2007-03-27 at 13:29 -0400, Luis R. Rodriguez wrote:
> > > On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> > > > On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> > > > > On 3/26/07, mohamed <[email protected]> wrote:
> > > > >
> > > > > > +/* Get 11n capabilties from low level driver */
> > > > > > +static void ieee80211_fill_ht_ie(struct net_device *dev,
> > > > > > + struct ieee80211_ht_capability *ht_capab)
> > > > > > +{
> > > > > > + int rc;
> > > > > > + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > > > > > +
> > > > > > + if (!local->ops->get_ht_capab){
> > > > > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > > > + return;
> > > > > > + }
> > > > > > +
> > > > > > + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> > > > > > + if (!rc) {
> > > > > > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > > > + return;
> > > > > > + }
> > > > > >
> > > > > > + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > > > > > + ht_capab->capabilitiesInfo);
> > > > > > + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > > > > > + ht_capab->extended_ht_capability_info);
> > > > > > + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > > > > > + ht_capab->tx_BF_capability_info);
> > > > > > +}
> > > > > > +
> > > > >
> > > > > We should memset to 0 the entire ht_capab to regardless as its coming
> > > > > from skb_put() otherwise we'll get random data there if we don't set
> > > > > it and we sure as hell don't want to just transmit that :)
> > I will memset to 0 before using the structure. I will change the patch
> > to reflect this.
>
> Thanks
>
> > > Also
> > > > > ieee80211_send_assoc() is already checking for
> > > > > local->ops->get_ht_capab so lets just do a BUG_ON here to capture
> > > > > cases where someone else didn't do the proper checking. So how about
> > > > > instead something like:
> > make sense.
>
> Thanks
>
> > > > > /* Get 11n capabilties from low level driver */
> > > > > static void ieee80211_fill_ht_ie(struct net_device *dev,
> > > > > struct ieee80211_ht_capability *ht_capab)
> > > > > {
> > > > > struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > > > > BUG_ON(!local->ops->get_ht_capab);
> > > > > memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > > > > if (!local->ops->get_ht_capab(local_to_hw(local), ht_capab))
> > > > > return;
> > > > > ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > > > > ht_capab->capabilitiesInfo);
> > > > > ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > > > > ht_capab->extended_ht_capability_info);
> > > > > ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > > > > ht_capab->tx_BF_capability_info);
> > > > > }
> > > > >
> > > >
> > > > On second thought just memset to 0 on ieee80211_send_assoc() right
> > > > after the skb_put and remove that from above.
> > > >
> > > > Luis
> > > >
> > >
> > > Last few comments:
> > >
> > > I. In struct ieee80211_ops you added your hunk after the TSF comment,
> > > but before u64 (*get_tsf)(struct ieee80211_hw *hw) was listed. Please
> > > move the hunk after get_tsf or before the comment.
> > >
> > > II. In ieee80211_fill_ht_ie() you do cpu_to_le* on the same fields.. I
> > > take it the driver's get_ht_capab() is supposed to fill these. Since
> > > you already defined them as little endian in the struct
> > > ieee80211_ht_capability, how about just leaving that up to the driver
> > > developer to make sure they do this? That's would be 6 lines less of
> > > code in ieee80211_fill_ht_ie().
>
> And how about regarding the above comment (II)? We can just expect
> driver developers to follow proper endianness. That cleans that
> routine a bit more too.
>
Yes I will do this as well
Mohamed
> Luis

2007-03-31 15:39:33

by Michael Wu

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Monday 26 March 2007 07:38, mohamed wrote:
> Add basic support for IEEE 802.11n discovery and association.
>
> This patch adds support to discover IEEE 802.11n AP and enable
> association to 802.11n Network. It parses beacon to discover 802.11n
> IE and include HT capability information element in Association Request
> Frame.
> It also call low level driver with the HT capability available during
> association.
>
So.. what happens when the user is using a userspace mlme?

-Michael Wu

Attachments:

2007-03-28 09:15:29

by Johannes Berg

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Mon, 2007-03-26 at 04:38 -0700, mohamed wrote:

> + /* Get ht capabilities from the device */
> + int (*get_ht_capab)(struct ieee80211_hw *hw,
> + struct ieee80211_ht_capability *ht_cap_param);
> +

Does this really need to return an error? And if so, what happens? You
currently send a blank HT capabilities IE but I think in that case it
should be left out completely. But I don't think it will ever return an
error, it doesn't even need to allocate memory. Just change this to
void?

johannes

Attachments:

2007-04-11 22:59:42

by Michael Wu

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Wednesday 11 April 2007 18:23, mabbas wrote:
> where i can get mac80211 userspace mlme?
> Mohamed
It's wpa_supplicant with mlme code turned on.

Git repo: git://w1.fi/srv/git/hostap.git

-Michael Wu

Attachments:

2007-04-11 22:23:06

by mabbas

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Michael Wu wrote:
> On Wednesday 11 April 2007 16:04, mabbas wrote:
>
>> I am not familiar with userspace mlme what needed to be done?
>>
> mac80211 has a userspace mlme mode which defers all things dealing with
> management frames to userspace (wpa_supplicant). It basically disables all
> the code in ieee80211_sta.c and routes management frames to a special
> interface where wpa_supplicant (or hostap) handles things. This is preferred
> and the ieee80211_sta.c code is there for backwards compatibility.
>
> No major features should be added to the in-kernel MLME (ieee80211_sta.c) but
> even more importantly, your patch series does not provide the appropriate
> hooks for a userspace MLME to support a driver using this 802.11n API.
>
> -Michael Wu
>
where i can get mac80211 userspace mlme?
Mohamed

2007-04-12 09:05:45

by Johannes Berg

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Thu, 2007-04-12 at 10:01 +0100, Andy Green wrote:

> Just curious, why is that evil? It seems if the usermode MLME is
> interested in what are basically Out Of Band 1Mbps management packets
> that share the same air as the payload packets and so appear on Monitor
> mode interfaces, then a local OOB class of packet doesn't break such new
> ground. And it does neatly fit the communication semantic already used.

Oh but those aren't *really* fake then. In this case, I'm ambivalent,
but the stuff we have in there for some things like radar notification
is just ugly.

johannes

Attachments:

2007-04-11 19:47:58

by John W. Linville

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Sat, Mar 31, 2007 at 11:37:19AM -0400, Michael Wu wrote:
> On Monday 26 March 2007 07:38, mohamed wrote:
> > Add basic support for IEEE 802.11n discovery and association.
> >
> > This patch adds support to discover IEEE 802.11n AP and enable
> > association to 802.11n Network. It parses beacon to discover 802.11n
> > IE and include HT capability information element in Association Request
> > Frame.
> > It also call low level driver with the HT capability available during
> > association.
> >
> So.. what happens when the user is using a userspace mlme?

So...Michael is unhappy that I merged this one (and maybe unhappy
about the whole series).

Mohamed, could you address his concern that your patch preclude
userland MLME functionality? Let's make sure we aren't closing-off
that capability.

Thanks,

John
--
John W. Linville
[email protected]

2007-04-11 23:03:35

by Simon Barber

Subject: RE: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Setup and tear down of BA should be done by userspace - actual
processing of the BA frames themselves should be done in mac80211. This
allows maximum flexibility from userspace to control the policy of when
to setup and teardown BA streams.

Simon

-----Original Message-----
From: Tomas Winkler [mailto:[email protected]]
Sent: Wednesday, April 11, 2007 3:04 PM
To: Michael Wu
Cc: mabbas; John W. Linville; [email protected]; Jouni
Malinen; Simon Barber
Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery
and association

On 4/11/07, Michael Wu <[email protected]> wrote:
> On Wednesday 11 April 2007 16:04, mabbas wrote:
> > I am not familiar with userspace mlme what needed to be done?
> mac80211 has a userspace mlme mode which defers all things dealing
> with management frames to userspace (wpa_supplicant). It basically
> disables all the code in ieee80211_sta.c and routes management frames
> to a special interface where wpa_supplicant (or hostap) handles
> things. This is preferred and the ieee80211_sta.c code is there for
backwards compatibility.
>
> No major features should be added to the in-kernel MLME
> (ieee80211_sta.c) but even more importantly, your patch series does
> not provide the appropriate hooks for a userspace MLME to support a
driver using this 802.11n API.
>
> -Michael Wu
>
>
Meanwhile I don't see feasible user space implementation for this as
data packet classification is done in kernel. BACK streams are
dynamically opened and teared down per TID according current traffic
shape.
This is new feature and the user space handling is not well understand
yet. I suggest that we proceed with patches and let the evolution making
its steps.

If someone has concrete suggestion on implementing it as user space MLME
I will be glad to hear.
Thanks


Tomas

2007-04-11 22:04:03

by Tomas Winkler

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On 4/11/07, Michael Wu <[email protected]> wrote:
> On Wednesday 11 April 2007 16:04, mabbas wrote:
> > I am not familiar with userspace mlme what needed to be done?
> mac80211 has a userspace mlme mode which defers all things dealing with
> management frames to userspace (wpa_supplicant). It basically disables all
> the code in ieee80211_sta.c and routes management frames to a special
> interface where wpa_supplicant (or hostap) handles things. This is preferred
> and the ieee80211_sta.c code is there for backwards compatibility.
>
> No major features should be added to the in-kernel MLME (ieee80211_sta.c) but
> even more importantly, your patch series does not provide the appropriate
> hooks for a userspace MLME to support a driver using this 802.11n API.
>
> -Michael Wu
>
>
Meanwhile I don't see feasible user space implementation for this as
data packet classification is done in kernel. BACK streams are
dynamically opened and teared down per TID according current traffic
shape.
This is new feature and the user space handling is not well understand
yet. I suggest that we proceed with patches and let the evolution
making its steps.

If someone has concrete suggestion on implementing it as user space
MLME I will be glad to hear.
Thanks


Tomas

2007-04-11 23:05:04

by Tomas Winkler

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

So you monitor your traffic load in driver, then notify user space
application which opens or tears down for you the BA stream?
What would be the notification mechanism ?


On 4/12/07, Simon Barber <[email protected]> wrote:
> Setup and tear down of BA should be done by userspace - actual
> processing of the BA frames themselves should be done in mac80211. This
> allows maximum flexibility from userspace to control the policy of when
> to setup and teardown BA streams.
>
> Simon
>
> -----Original Message-----
> From: Tomas Winkler [mailto:[email protected]]
> Sent: Wednesday, April 11, 2007 3:04 PM
> To: Michael Wu
> Cc: mabbas; John W. Linville; [email protected]; Jouni
> Malinen; Simon Barber
> Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery
> and association
>
> On 4/11/07, Michael Wu <[email protected]> wrote:
> > On Wednesday 11 April 2007 16:04, mabbas wrote:
> > > I am not familiar with userspace mlme what needed to be done?
> > mac80211 has a userspace mlme mode which defers all things dealing
> > with management frames to userspace (wpa_supplicant). It basically
> > disables all the code in ieee80211_sta.c and routes management frames
> > to a special interface where wpa_supplicant (or hostap) handles
> > things. This is preferred and the ieee80211_sta.c code is there for
> backwards compatibility.
> >
> > No major features should be added to the in-kernel MLME
> > (ieee80211_sta.c) but even more importantly, your patch series does
> > not provide the appropriate hooks for a userspace MLME to support a
> driver using this 802.11n API.
> >
> > -Michael Wu
> >
> >
> Meanwhile I don't see feasible user space implementation for this as
> data packet classification is done in kernel. BACK streams are
> dynamically opened and teared down per TID according current traffic
> shape.
> This is new feature and the user space handling is not well understand
> yet. I suggest that we proceed with patches and let the evolution making
> its steps.
>
> If someone has concrete suggestion on implementing it as user space MLME
> I will be glad to hear.
> Thanks
>
>
> Tomas
>

2007-04-11 20:33:26

by Michael Wu

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Wednesday 11 April 2007 16:04, mabbas wrote:
> I am not familiar with userspace mlme what needed to be done?
mac80211 has a userspace mlme mode which defers all things dealing with
management frames to userspace (wpa_supplicant). It basically disables all
the code in ieee80211_sta.c and routes management frames to a special
interface where wpa_supplicant (or hostap) handles things. This is preferred
and the ieee80211_sta.c code is there for backwards compatibility.

No major features should be added to the in-kernel MLME (ieee80211_sta.c) but
even more importantly, your patch series does not provide the appropriate
hooks for a userspace MLME to support a driver using this 802.11n API.

-Michael Wu

Attachments:

2007-04-12 18:12:01

by mabbas

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Michael Wu wrote:
> On Wednesday 11 April 2007 18:23, mabbas wrote:
>
>> where i can get mac80211 userspace mlme?
>> Mohamed
>>
> It's wpa_supplicant with mlme code turned on.
>
> Git repo: git://w1.fi/srv/git/hostap.git
>
> -Michael
just adding the hooks to userspace mlme would be enough to get these
patches accepted. then I can look into wpa_supplicant.
Mohamed
>
>

2007-04-11 20:04:25

by mabbas

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

John W. Linville wrote:
> On Sat, Mar 31, 2007 at 11:37:19AM -0400, Michael Wu wrote:
>
>> On Monday 26 March 2007 07:38, mohamed wrote:
>>
>>> Add basic support for IEEE 802.11n discovery and association.
>>>
>>> This patch adds support to discover IEEE 802.11n AP and enable
>>> association to 802.11n Network. It parses beacon to discover 802.11n
>>> IE and include HT capability information element in Association Request
>>> Frame.
>>> It also call low level driver with the HT capability available during
>>> association.
>>>
>>>
>> So.. what happens when the user is using a userspace mlme?
>>
I am not familiar with userspace mlme what needed to be done?
Mohamed
>
> So...Michael is unhappy that I merged this one (and maybe unhappy
> about the whole series).
>
> Mohamed, could you address his concern that your patch preclude
> userland MLME functionality? Let's make sure we aren't closing-off
> that capability.
>
> Thanks,
>
> John
>

2007-04-11 23:25:08

by Andy Green

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Tomas Winkler wrote:

> So you monitor your traffic load in driver, then notify user space
> application which opens or tears down for you the BA stream?
> What would be the notification mechanism ?

Since hopefully the concept will involve a kernelside-filtered
monitoring action via libpcap, the stack could for example inject a
periodic stats summary packet down the monitor interface back to userspace.

-Andy

2007-04-12 00:18:57

by Simon Barber

Subject: RE: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

There are several possible notification mechanisms - according to the
application's needs. It could be part of tc - using a qdisc, or libpcap,
or something hardwired into mac80211 or something else. Someone might
want this controlled by a userspace application (e.g. video streaming).
Someone else might want an automatic algorithm.

Simon

-----Original Message-----
From: Tomas Winkler [mailto:[email protected]]
Sent: Wednesday, April 11, 2007 4:05 PM
To: Simon Barber
Cc: Michael Wu; mabbas; John W. Linville;
[email protected]; Jouni Malinen
Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery
and association

So you monitor your traffic load in driver, then notify user space
application which opens or tears down for you the BA stream?
What would be the notification mechanism ?


On 4/12/07, Simon Barber <[email protected]> wrote:
> Setup and tear down of BA should be done by userspace - actual
> processing of the BA frames themselves should be done in mac80211.
> This allows maximum flexibility from userspace to control the policy
> of when to setup and teardown BA streams.
>
> Simon
>
> -----Original Message-----
> From: Tomas Winkler [mailto:[email protected]]
> Sent: Wednesday, April 11, 2007 3:04 PM
> To: Michael Wu
> Cc: mabbas; John W. Linville; [email protected]; Jouni
> Malinen; Simon Barber
> Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery
> and association
>
> On 4/11/07, Michael Wu <[email protected]> wrote:
> > On Wednesday 11 April 2007 16:04, mabbas wrote:
> > > I am not familiar with userspace mlme what needed to be done?
> > mac80211 has a userspace mlme mode which defers all things dealing
> > with management frames to userspace (wpa_supplicant). It basically
> > disables all the code in ieee80211_sta.c and routes management
> > frames to a special interface where wpa_supplicant (or hostap)
> > handles things. This is preferred and the ieee80211_sta.c code is
> > there for
> backwards compatibility.
> >
> > No major features should be added to the in-kernel MLME
> > (ieee80211_sta.c) but even more importantly, your patch series does
> > not provide the appropriate hooks for a userspace MLME to support a
> driver using this 802.11n API.
> >
> > -Michael Wu
> >
> >
> Meanwhile I don't see feasible user space implementation for this as
> data packet classification is done in kernel. BACK streams are
> dynamically opened and teared down per TID according current traffic
> shape.
> This is new feature and the user space handling is not well understand

> yet. I suggest that we proceed with patches and let the evolution
> making its steps.
>
> If someone has concrete suggestion on implementing it as user space
> MLME I will be glad to hear.
> Thanks
>
>
> Tomas
>

2007-04-12 09:02:03

by Andy Green

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Johannes Berg wrote:
> On Thu, 2007-04-12 at 00:25 +0100, Andy Green wrote:
>
>> Since hopefully the concept will involve a kernelside-filtered
>> monitoring action via libpcap, the stack could for example inject a
>> periodic stats summary packet down the monitor interface back to userspace.
>
> Eek. No *new* fake packets any more please.

Just curious, why is that evil? It seems if the usermode MLME is
interested in what are basically Out Of Band 1Mbps management packets
that share the same air as the payload packets and so appear on Monitor
mode interfaces, then a local OOB class of packet doesn't break such new
ground. And it does neatly fit the communication semantic already used.

-Andy

2007-04-03 22:36:15

by mabbas

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

modified patch at the end

Randy Dunlap wrote:
>
>
> I'm seeing an email/patch problem here. The lines above should be
> 3 lines only, not 6 that I'm seeing. Maybe it's my mail client,
> but it usually works for me, so possibly yours is splitting lines
> when it should not.
>
>
>> @TSF timer value from firmware/hardware. Currently,
>> * this is only used for IBSS mode debugging and, as such, is not a
>> * required function. */
>> +
>> + /* Configure ht parameters
>>
>
> Line above ends with space. :(
>
> What is "ht"?
>
>
>> return cw - 1;
>> }
>>
>> +/* call low level driver with 11n params as it was recieved
>>
>
> received
>
>> + from the AP
>> +*/
>>
>
> Line above ends with spaces.
>
> Kernel long comment style is:
>
> /*
> * call low-level driver with 11n params as they were received
> * from the AP
> */
>
>
>> struct ieee80211_if_sta *ifsta,
>> u8 *wmm_param, size_t wmm_param_len)
>> @@ -584,6 +648,15 @@ static void ieee80211_send_assoc(struct
>> *pos++ = 0;
>> }
>>
>> + /* if low level driver support 11n fill in 11n IE */
>>
>
> supports 11n,
>
>
>> + if (ht_enabled && ifsta->ht_enabled && local->ops->get_ht_capab) {
>> + pos = skb_put(skb, sizeof(struct ieee80211_ht_capability)+2);
>> + *pos++ = WLAN_EID_HT_CAPABILITY;
>> + *pos++ = sizeof(struct ieee80211_ht_capability);
>> + ieee80211_fill_ht_ie(dev,
>> + (struct ieee80211_ht_capability *)pos);
>> + }
>> +
>> kfree(ifsta->assocreq_ies);
>> ifsta->assocreq_ies_len = (skb->data + skb->len) - ies;
>> ifsta->assocreq_ies = kmalloc(ifsta->assocreq_ies_len, GFP_ATOMIC);
>>
>
>
On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
On 3/27/07, Luis R. Rodriguez <[email protected]> wrote:
> On 3/26/07, mohamed <[email protected]> wrote:
>
> > +/* Get 11n capabilties from low level driver */
> > +static void ieee80211_fill_ht_ie(struct net_device *dev,
> > + struct ieee80211_ht_capability *ht_capab)
> > +{
> > + int rc;
> > + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > +
> > + if (!local->ops->get_ht_capab){
> > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > + return;
> > + }
> > +
> > + rc = local->ops->get_ht_capab(local_to_hw(local), ht_capab);
> > + if (!rc) {
> > + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> > + return;
> > + }
> >
> > + ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> > + ht_capab->capabilitiesInfo);
> > + ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> > + ht_capab->extended_ht_capability_info);
> > + ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> > + ht_capab->tx_BF_capability_info);
> > +}
> > +
>
> We should memset to 0 the entire ht_capab to regardless as its coming
> from skb_put() otherwise we'll get random data there if we don't set
> it and we sure as hell don't want to just transmit that :) Also
> ieee80211_send_assoc() is already checking for
> local->ops->get_ht_capab so lets just do a BUG_ON here to capture
> cases where someone else didn't do the proper checking. So how about
> instead something like:
>
> /* Get 11n capabilties from low level driver */
> static void ieee80211_fill_ht_ie(struct net_device *dev,
> struct ieee80211_ht_capability *ht_capab)
> {
> struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> BUG_ON(!local->ops->get_ht_capab);
> memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
> if (!local->ops->get_ht_capab(local_to_hw(local), ht_capab))
> return;
> ht_capab->capabilitiesInfo = (__le16) cpu_to_le16(
> ht_capab->capabilitiesInfo);
> ht_capab->extended_ht_capability_info = (__le16) cpu_to_le16(
> ht_capab->extended_ht_capability_info);
> ht_capab->tx_BF_capability_info = (__le32) cpu_to_le32(
> ht_capab->tx_BF_capability_info);
> }
>

> On second thought just memset to 0 on ieee80211_send_assoc() right
> after the skb_put and remove that from above.

Johannes Berg wrote:
> On Mon, 2007-03-26 at 04:38 -0700, mohamed wrote:
>
>
>> @@ -96,6 +96,10 @@ struct ieee802_11_elems {
>> u8 rsn_len;
>> u8 *erp_info;
>> u8 erp_info_len;
>> + u8 *ht_cap_param;
>> + u8 ht_cap_param_len;
>> + u8 *ht_extra_param;
>> + u8 ht_extra_param_len;
>> u8 *ext_supp_rates;
>> u8 ext_supp_rates_len;
>> u8 *wmm_info;
>>
>
> Random note not applicable to your patch: This is quite a large struct,
> especially on 64-bit. Maybe we should be thinking about making those
> pointers there u16 offsets instead. And we *definitely* should change
> the order of these fields, having u8 and u8* alternate just sucks.
>
>
>> +static void ieee80211_sta_ht_params(struct net_device *dev,
>> + struct sta_info *sta,
>> + struct ieee80211_ht_additional_info *ht_extra_param,
>> + struct ieee80211_ht_capability *ht_cap_param)
>> +{
>> + int rc;
>> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
>> +
>> + if (local->ops->conf_ht) {
>> + rc = local->ops->conf_ht(local_to_hw(local), ht_cap_param,
>> + ht_extra_param);
>> +
>> + if (rc)
>> + sta->flags &= ~WLAN_STA_HT;
>> + } else
>> + sta->flags &= ~WLAN_STA_HT;
>>
>
> Shouldn't this also set the STA_HT flag in the case where conf_ht
> returns without error?
>
>
>> +/* Get 11n capabilties from low level driver */
>> +static void ieee80211_fill_ht_ie(struct net_device *dev,
>>
>
> [filling in how it looks like after Luis's and my proposed changes]
>
>
>> + struct ieee80211_ht_capability *ht_capab)
>> +{
>> + struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
>> + BUG_ON(!local->ops->get_ht_capab);
>> + memset(ht_capab, 0, sizeof(struct ieee80211_ht_capability));
>> + local->ops->get_ht_capab(local_to_hw(local), ht_capab);
>>
>
> some blank lines would be good ;)
>
> Maybe we should be setting some fields to default values here that
> aren't zero? Haven't checked.
>
>
>> + /* if low level driver support 11n fill in 11n IE */
>> + if (ht_enabled && ifsta->ht_enabled && local->ops->get_ht_capab) {
>> + pos = skb_put(skb, sizeof(struct ieee80211_ht_capability)+2);
>> + *pos++ = WLAN_EID_HT_CAPABILITY;
>> + *pos++ = sizeof(struct ieee80211_ht_capability);
>> + ieee80211_fill_ht_ie(dev,
>> + (struct ieee80211_ht_capability *)pos);
>> Now that fill_ht_ie is so short maybe it should just be rolled into this
>>
Add basic support for IEEE 802.11n discovery and association.

This patch adds support to discover IEEE 802.11n AP and enable
association to 802.11n Network. It parses beacon to discover 802.11n
IE and include HT capability information element in Association Request Frame.
It also call low level driver with the HT capability available during
association.
Tomas:

1. Removed trailing spaces
2. tsf handlers declarations not splited by ht hanlders
2. fill_ht and ht_params functions removed
3. Moved code from sta_sysfs.c to debug_sta.c

Signed-off-by: Mohamed Abbas <[email protected]>

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 916b21b..b1bbc3d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -529,6 +529,9 @@ #define IEEE80211_HW_TKIP_REQ_PHASE1_KEY
* per-packet RC4 key with each TX frame when doing hwcrypto */
#define IEEE80211_HW_TKIP_REQ_PHASE2_KEY (1<<14)

+ /* The device capable of supporting 11n */
+#define IEEE80211_HW_SUPPORT_HT_MODE (1<<15)
+
u32 flags; /* hardware flags defined above */

/* Set to the size of a needed device specific skb headroom for TX skbs. */
@@ -731,6 +734,15 @@ struct ieee80211_ops {
* TSF synchronization. */
void (*reset_tsf)(struct ieee80211_hw *hw);

+ /* Configure ht parameters. */
+ int (*conf_ht)(struct ieee80211_hw *hw,
+ struct ieee80211_ht_capability *ht_cap_param,
+ struct ieee80211_ht_additional_info *ht_extra_param);
+
+ /* Get ht capabilities from the device */
+ void (*get_ht_capab)(struct ieee80211_hw *hw,
+ struct ieee80211_ht_capability *ht_cap_param);
+
/* Setup beacon data for IBSS beacons. Unlike access point (Master),
* IBSS uses a fixed beacon frame which is configured using this
* function. This handler is required only for IBSS mode. */
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 3012b9e..aa3035b 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -87,7 +87,7 @@ static ssize_t sta_flags_read(struct fil
{
char buf[100];
struct sta_info *sta = file->private_data;
- int res = scnprintf(buf, sizeof(buf), "%s%s%s%s%s%s%s%s%s",
+ int res = scnprintf(buf, sizeof(buf), "%s%s%s%s%s%s%s%s%s%s",
sta->flags & WLAN_STA_AUTH ? "AUTH\n" : "",
sta->flags & WLAN_STA_ASSOC ? "ASSOC\n" : "",
sta->flags & WLAN_STA_PS ? "PS\n" : "",
@@ -96,6 +96,7 @@ static ssize_t sta_flags_read(struct fil
sta->flags & WLAN_STA_AUTHORIZED ? "AUTHORIZED\n" : "",
sta->flags & WLAN_STA_SHORT_PREAMBLE ? "SHORT PREAMBLE\n" : "",
sta->flags & WLAN_STA_WME ? "WME\n" : "",
+ sta->flags & WLAN_STA_HT ? "HT\n" : "",
sta->flags & WLAN_STA_WDS ? "WDS\n" : "");
return simple_read_from_buffer(userbuf, count, ppos, buf, res);
}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e8eea4d..dd1d031 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -89,6 +89,8 @@ struct ieee80211_sta_bss {
size_t rsn_ie_len;
u8 *wmm_ie;
size_t wmm_ie_len;
+ u8 *ht_ie;
+ size_t ht_ie_len;
#define IEEE80211_MAX_SUPP_RATES 32
u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
size_t supp_rates_len;
@@ -268,6 +270,7 @@ struct ieee80211_if_sta {
unsigned int create_ibss:1;
unsigned int mixed_cell:1;
unsigned int wmm_enabled:1;
+ unsigned int ht_enabled:1;
unsigned int auto_ssid_sel:1;
unsigned int auto_bssid_sel:1;
unsigned int auto_channel_sel:1;
diff --git a/net/mac80211/ieee80211_iface.c b/net/mac80211/ieee80211_iface.c
index b1b20ba..ebea50d 100644
--- a/net/mac80211/ieee80211_iface.c
+++ b/net/mac80211/ieee80211_iface.c
@@ -184,6 +184,7 @@ void ieee80211_if_set_type(struct net_de
IEEE80211_AUTH_ALG_SHARED_KEY;
ifsta->create_ibss = 1;
ifsta->wmm_enabled = 1;
+ ifsta->ht_enabled = 1;
ifsta->auto_channel_sel = 1;
ifsta->auto_bssid_sel = 1;

diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index 087f176..2ae0a56 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -96,6 +96,10 @@ struct ieee802_11_elems {
u8 rsn_len;
u8 *erp_info;
u8 erp_info_len;
+ u8 *ht_cap_param;
+ u8 ht_cap_param_len;
+ u8 *ht_extra_param;
+ u8 ht_extra_param_len;
u8 *ext_supp_rates;
u8 ext_supp_rates_len;
u8 *wmm_info;
@@ -197,6 +201,14 @@ #endif
elems->ext_supp_rates = pos;
elems->ext_supp_rates_len = elen;
break;
+ case WLAN_EID_HT_CAPABILITY:
+ elems->ht_cap_param = pos;
+ elems->ht_cap_param_len = elen;
+ break;
+ case WLAN_EID_HT_EXTRA_INFO:
+ elems->ht_extra_param = pos;
+ elems->ht_extra_param_len = elen;
+ break;
default:
#if 0
printk(KERN_DEBUG "IEEE 802.11 element parse ignored "
@@ -230,7 +242,6 @@ static int ecw2cw(int ecw)
return cw - 1;
}

-
static void ieee80211_sta_wmm_params(struct net_device *dev,
struct ieee80211_if_sta *ifsta,
u8 *wmm_param, size_t wmm_param_len)
@@ -490,6 +501,7 @@ static void ieee80211_send_assoc(struct
u16 capab;
struct ieee80211_sta_bss *bss;
int wmm = 0;
+ int ht_enabled = 0;

skb = dev_alloc_skb(local->hw.extra_tx_headroom +
sizeof(*mgmt) + 200 + ifsta->extra_ie_len +
@@ -513,6 +525,8 @@ static void ieee80211_send_assoc(struct
capab |= WLAN_CAPABILITY_PRIVACY;
if (bss->wmm_ie) {
wmm = 1;
+
+ ht_enabled = 1;
}
ieee80211_rx_bss_put(dev, bss);
}
@@ -588,6 +602,16 @@ static void ieee80211_send_assoc(struct
*pos++ = 0;
}

+ /* if low level driver supports 11n, fill in 11n IE */
+ if (ht_enabled && ifsta->ht_enabled && local->ops->get_ht_capab) {
+ pos = skb_put(skb, sizeof(struct ieee80211_ht_capability)+2);
+ *pos++ = WLAN_EID_HT_CAPABILITY;
+ *pos++ = sizeof(struct ieee80211_ht_capability);
+ memset(pos, 0, sizeof(struct ieee80211_ht_capability));
+ local->ops->get_ht_capab(local_to_hw(local),
+ (struct ieee80211_ht_capability *)pos);
+ }
+
kfree(ifsta->assocreq_ies);
ifsta->assocreq_ies_len = (skb->data + skb->len) - ies;
ifsta->assocreq_ies = kmalloc(ifsta->assocreq_ies_len, GFP_ATOMIC);
@@ -1223,6 +1247,20 @@ static void ieee80211_rx_mgmt_assoc_resp
}
sta->supp_rates = rates;

+ if (elems.ht_extra_param && elems.ht_cap_param && elems.wmm_param &&
+ ifsta->ht_enabled && local->ops->conf_ht){
+ int rc;
+
+ rc = local->ops->conf_ht(local_to_hw(local),
+ (struct ieee80211_ht_capability *)
+ elems.ht_cap_param,
+ (struct ieee80211_ht_additional_info *)
+ elems.ht_extra_param);
+ if (!rc)
+ sta->flags |= WLAN_STA_HT;
+ }
+
+
rate_control_rate_init(sta, local);

if (elems.wmm_param && ifsta->wmm_enabled) {
@@ -1318,6 +1356,7 @@ static void ieee80211_rx_bss_free(struct
kfree(bss->wpa_ie);
kfree(bss->rsn_ie);
kfree(bss->wmm_ie);
+ kfree(bss->ht_ie);
kfree(bss);
}

@@ -1564,6 +1603,23 @@ #endif
bss->wmm_ie_len = 0;
}

+ if (elems.ht_cap_param &&
+ (!bss->ht_ie || bss->ht_ie_len != elems.ht_cap_param_len ||
+ memcmp(bss->ht_ie, elems.ht_cap_param, elems.ht_cap_param_len))) {
+ if (bss->ht_ie)
+ kfree(bss->ht_ie);
+ bss->ht_ie = kmalloc(elems.ht_cap_param_len + 2, GFP_ATOMIC);
+ if (bss->ht_ie) {
+ memcpy(bss->ht_ie, elems.ht_cap_param - 2,
+ elems.ht_cap_param_len + 2);
+ bss->ht_ie_len = elems.ht_cap_param_len + 2;
+ } else
+ bss->ht_ie_len = 0;
+ } else if (!elems.ht_cap_param && bss->ht_ie) {
+ kfree(bss->ht_ie);
+ bss->ht_ie = NULL;
+ bss->ht_ie_len = 0;
+ }

bss->hw_mode = rx_status->phymode;
bss->channel = channel;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index ed7ca59..0010fde 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -27,6 +27,7 @@ #define WLAN_STA_AUTHORIZED BIT(5) /* If
*/
#define WLAN_STA_SHORT_PREAMBLE BIT(7)
#define WLAN_STA_WME BIT(9)
+#define WLAN_STA_HT BIT(10)
#define WLAN_STA_WDS BIT(27)

2007-04-12 08:55:30

by Johannes Berg

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

On Thu, 2007-04-12 at 00:25 +0100, Andy Green wrote:

> Since hopefully the concept will involve a kernelside-filtered
> monitoring action via libpcap, the stack could for example inject a
> periodic stats summary packet down the monitor interface back to userspace.

Eek. No *new* fake packets any more please.

johannes

Attachments:

2007-04-12 01:13:13

by Tomas Winkler

Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery and association

Actually the recipient part I can see be implemented in the user
space MLME. The initiator part requires request from kernel to user
space to start the BA negotiation.
Still there is some amount internal state information that need to be
passed back and forth that make user space MLME to be a long shot, bat
eventually we get there.
Dynamic algorithm look like best option to me as keeping underutilized
stream only hurts the performance. User application may tune
thresholds and timeouts and of course must tag the traffic.


On 4/12/07, Simon Barber <[email protected]> wrote:
> There are several possible notification mechanisms - according to the
> application's needs. It could be part of tc - using a qdisc, or libpcap,
> or something hardwired into mac80211 or something else. Someone might
> want this controlled by a userspace application (e.g. video streaming).
> Someone else might want an automatic algorithm.
>
> Simon
>
> -----Original Message-----
> From: Tomas Winkler [mailto:[email protected]]
> Sent: Wednesday, April 11, 2007 4:05 PM
> To: Simon Barber
> Cc: Michael Wu; mabbas; John W. Linville;
> [email protected]; Jouni Malinen
> Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery
> and association
>
> So you monitor your traffic load in driver, then notify user space
> application which opens or tears down for you the BA stream?
> What would be the notification mechanism ?
>
>
> On 4/12/07, Simon Barber <[email protected]> wrote:
> > Setup and tear down of BA should be done by userspace - actual
> > processing of the BA frames themselves should be done in mac80211.
> > This allows maximum flexibility from userspace to control the policy
> > of when to setup and teardown BA streams.
> >
> > Simon
> >
> > -----Original Message-----
> > From: Tomas Winkler [mailto:[email protected]]
> > Sent: Wednesday, April 11, 2007 3:04 PM
> > To: Michael Wu
> > Cc: mabbas; John W. Linville; [email protected]; Jouni
> > Malinen; Simon Barber
> > Subject: Re: [patch 2/5] Add basic support for IEEE 802.11n discovery
> > and association
> >
> > On 4/11/07, Michael Wu <[email protected]> wrote:
> > > On Wednesday 11 April 2007 16:04, mabbas wrote:
> > > > I am not familiar with userspace mlme what needed to be done?
> > > mac80211 has a userspace mlme mode which defers all things dealing
> > > with management frames to userspace (wpa_supplicant). It basically
> > > disables all the code in ieee80211_sta.c and routes management
> > > frames to a special interface where wpa_supplicant (or hostap)
> > > handles things. This is preferred and the ieee80211_sta.c code is
> > > there for
> > backwards compatibility.
> > >
> > > No major features should be added to the in-kernel MLME
> > > (ieee80211_sta.c) but even more importantly, your patch series does
> > > not provide the appropriate hooks for a userspace MLME to support a
> > driver using this 802.11n API.
> > >
> > > -Michael Wu
> > >
> > >
> > Meanwhile I don't see feasible user space implementation for this as
> > data packet classification is done in kernel. BACK streams are
> > dynamically opened and teared down per TID according current traffic
> > shape.
> > This is new feature and the user space handling is not well understand
>
> > yet. I suggest that we proceed with patches and let the evolution
> > making its steps.
> >
> > If someone has concrete suggestion on implementing it as user space
> > MLME I will be glad to hear.
> > Thanks
> >
> >
> > Tomas
> >
>