Search Linux Wireless

Re: [PATCH v2] mac80211: Add HT IE to IBSS beacons and probe responses.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 06/05/2010 08:40, Johannes Berg a écrit :
On Thu, 2010-05-06 at 00:36 +0200, Benoit Papillault wrote:
When an HT IBSS is configured, we add HT Information and HT Capabilities
IE to beacons&  probe responses. This is done according to channel_type
transmitted by iw/cfg80211.

v2: Added helpers to build HT Capability&  HT Information IE

This line belongs after the --- so it's not part of the commit log.

  	local->oper_channel = chan;
-	local->oper_channel_type = NL80211_CHAN_NO_HT;
+	local->oper_channel_type = channel_type;

It would be helpful if you were to rebase over my patch that adds the
channel type tracking.

Your patches are now in wireless-testing, so I am doing it at the moment.


@@ -165,6 +169,14 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
  		memcpy(pos,&supp_rates[8], rates);
  	}

+	if (channel_type != NL80211_CHAN_NO_HT&&
+	    sband->ht_cap.ht_supported) {

You shouldn't be able to get here with an HT channel but !ht_supported,
no? Or is that to support the case where you have HT only on one band?

Good point. I think there are several cases here :

- a non-HT STA is joining an HT IBSS (we need to check 802.11n-2009 to see how it's supposed to be handled). In this case channel_type could be ht40+ and ht_supported = false

- an HT STA is joining a non-HT IBSS. It's clear in this case that no HT IE should be sent, which is catched by (channel_type != NL80211_CHAN_NO_HT) condition.

Could we examine those cases in a follow up patch?


+	}
+	

trailing whitespace

@@ -202,6 +214,9 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
  	u32 basic_rates;
  	int i, j;
  	u16 beacon_int = cbss->beacon_interval;
+	const u8 * ht_info_ie;

CodingStyle.

@@ -223,9 +238,28 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
  		}
  	}

+	/* parse HT Information IE, if present */
+	ht_info_ie = ieee80211_bss_get_ie(cbss, WLAN_EID_HT_INFORMATION);
+	if (ht_info_ie) {
+		ht_info = (const struct ieee80211_ht_info *)(ht_info_ie + 2);
+		switch (ht_info->ht_param
+			&  IEEE80211_HT_PARAM_CHA_SEC_OFFSET) {

The parser in mlme.c looks different. Please see there and also make it
a helper function.

Thanks. I did not notice.


--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -404,6 +404,7 @@ struct ieee80211_if_ibss {
  	u8 ssid_len, ie_len;
  	u8 *ie;
  	struct ieee80211_channel *channel;
+	enum nl80211_channel_type channel_type;

  	unsigned long ibss_join_req;
  	/* probe response/beacon for IBSS */
@@ -1193,6 +1194,15 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
  			 u16 transaction, u16 auth_alg,
  			 u8 *extra, size_t extra_len, const u8 *bssid,
  			 const u8 *key, u8 key_len, u8 key_idx);
+
+int ieee80211_add_ht_cap(u8 **ppos,
+			 struct ieee80211_supported_band *sband);

The length is fixed anyway, so how about just passing in
	...(u8 *buffer, ... sband)

+int ieee80211_add_ht_info(u8 **ppos,
+			  struct ieee80211_supported_band *sband,
+			  struct ieee80211_channel *channel,
+			  enum nl80211_channel_type channel_type);

same here.

--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -898,6 +898,82 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
  	ieee80211_tx_skb(sdata, skb);
  }

+int ieee80211_add_ht_cap(u8 **ppos,
+			 struct ieee80211_supported_band *sband)
+{

Actually ... please split up the patches.

  1) pure code moving from the mlme code to helper functions
  2) helper function rewrite using the structures, like you did
  3) this patch

+	u16 cap = sband->ht_cap.cap;
+	struct ieee80211_ht_cap ht_cap;
+	u8 *pos = *ppos;
+
+	if (ieee80211_disable_40mhz_24ghz&&
+	    sband->band == IEEE80211_BAND_2GHZ) {
+		cap&= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+		cap&= ~IEEE80211_HT_CAP_SGI_40;
+	}
+
+	ht_cap.cap_info = cpu_to_le16(cap);
+	ht_cap.ampdu_params_info = sband->ht_cap.ampdu_factor |
+		(sband->ht_cap.ampdu_density<<
+		 IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
+	ht_cap.mcs = sband->ht_cap.mcs;
+	ht_cap.extended_ht_cap_info = cpu_to_le16(0);
+	ht_cap.tx_BF_cap_info = cpu_to_le32(0);
+	ht_cap.antenna_selection_info = 0;
+
+	/* HT Capabilities element */
+	*pos++ = WLAN_EID_HT_CAPABILITY;
+	*pos++ = sizeof(struct ieee80211_ht_cap);
+	memcpy(pos,&ht_cap, sizeof(struct ieee80211_ht_cap));
+	pos += sizeof(struct ieee80211_ht_cap);
+
+	*ppos = pos;
+
+	return 1;

oh and get rid of the entirely useless return value.

Let's do so.


+}
+
+int ieee80211_add_ht_info(u8 **ppos,
+			  struct ieee80211_supported_band *sband,
+			  struct ieee80211_channel *channel,
+			  enum nl80211_channel_type channel_type)

what's wrong with ieee80211_add_ht_ie()

Seems it's close to what I'm doing, but not entirely the same. I will read it. If applicable, should I move ieee80211_add_ht_ie to util.c (and declaration to ieee80211_i.h) then ?


Some more effort please! Patches with such _basic_ errors are no fun to
review!

I will try to do better next time, sorry about that.


johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux