Search Linux Wireless

Re: [PATCH v4 2/2] mac80211: Support ht-cap over-rides.

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

 



On 11/07/2011 01:13 AM, Johannes Berg wrote:
On Fri, 2011-11-04 at 13:10 -0700, greearb@xxxxxxxxxxxxxxx wrote:

+/*
+ * Stations supporting 802.11n are required to support
+ * at least the first 8 MCS rates.  See section 7.3.2.56.4
+ * and 20.1.1 of the 802.11n spec.
+ */
+#define IEEE80211_HT_MCS_REQ_RATES_STA		8

I'd prefer if this was a validation on the input from userspace directly
in cfg80211, that way other drivers that want to implement this don't
have to bother.

That probably goes well with the validation of the supported mask that I
asked for.

It is valid to request that the station use a lesser rate, it just
needs to advertise that it supports the first 8 rates.  So, for the
rate-control logic, we use exactly what is passed in from user-space,
but when generating the HT-caps objects, we set a minimum of 8 MCS
rates.

So, if you want validation, then I'm going to have to add a second
mcs argument and mask to distinguish between what we should advertise
v/s what we should use to over-ride the local rate-control mechanism.

I will do this if you ask, but in my opinion, the current functionality
is good enough.

+	/* mac80211 allows over-riding some of the ht-capabilities */
+	local->hw.wiphy->ht_capa_mod_mask =
+		kzalloc(sizeof(*local->hw.wiphy->ht_capa_mod_mask),
+			GFP_KERNEL);
+	if (local->hw.wiphy->ht_capa_mod_mask) {
+		struct ieee80211_ht_cap *cm = local->hw.wiphy->ht_capa_mod_mask;
+		u8 *r = (u8 *)(&cm->mcs.rx_mask);
+		memset(r, 0xff, IEEE80211_HT_MCS_MASK_LEN);
+		cm->cap_info |= IEEE80211_HT_CAP_MAX_AMSDU;
+		cm->ampdu_params_info |= IEEE80211_HT_AMPDU_PARM_FACTOR;
+		cm->ampdu_params_info |= IEEE80211_HT_AMPDU_PARM_DENSITY;
+	}

Why is this not just a static const that you fill manually? There's
nothing that's not constant here. So e.g.

static const struct ieee80211_ht_cap mac80211_ht_capa_mod_mask = {
	.ampdu_params_info = IEEE80211_HT_AMPDU_PARM_FACTOR |
			     IEEE80211_HT_AMPDU_PARM_DENSITY,
	.mcs = {
		.rx_mask = { 0xff, 0xff, 0xff, 0xff, 0xff,
			     0xff, 0xff, 0xff, 0xff, 0xff, },
	},
	/* etc */
};

Well, you suggested a pointer in the wiphy struct that was null
for non mac80211 interfaces.  I'm not sure how to distinguish between
mac80211 and other wiphys when reporting the capabilities if I use
this global static.  I also like that the non-static logic lets
us tweak this for individual drivers if that becomes an issue.

@@ -988,6 +1001,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
   fail_wiphy_register:
  	if (local->wiphy_ciphers_allocated)
  		kfree(local->hw.wiphy->cipher_suites);
+	kfree(local->hw.wiphy->ht_capa_mod_mask);
+	local->hw.wiphy->ht_capa_mod_mask = NULL;

No need for this then obviously.


Looks pretty good overall. This is _much_ more readable than the
previous series. :-)

Thanks...making progress :)

Ben


johannes


--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc  http://www.candelatech.com
--
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