Patchwork wpa_supplicant: Allow user to disable short guard interval (SGI).

login
register
mail settings
Submitter Ben Greear
Date Dec. 13, 2012, 5:30 p.m.
Message ID <1355419826-4070-1-git-send-email-greearb@candelatech.com>
Download mbox | patch
Permalink /patch/206201/
State Accepted
Commit a90497f85f6820ec58ab1aefc9247ad38b9d83e9
Headers show

Comments

Ben Greear - Dec. 13, 2012, 5:30 p.m.
From: Ben Greear <greearb@candelatech.com>

Requires kernel patch to make the SGI-20 properly disabled...SGI-40
will already work since kernel 3.4 or so.

Signed-hostap: Ben Greear <greearb@candelatech.com>
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 wpa_supplicant/config.c         |    2 ++
 wpa_supplicant/config_ssid.h    |    9 +++++++++
 wpa_supplicant/wpa_supplicant.c |   23 ++++++++++++++++++++++-
 3 files changed, 33 insertions(+), 1 deletions(-)
Jouni Malinen - Dec. 18, 2012, 12:46 p.m.
On Thu, Dec 13, 2012 at 09:30:26AM -0800, greearb@candelatech.com wrote:
> Requires kernel patch to make the SGI-20 properly disabled...SGI-40
> will already work since kernel 3.4 or so.

Thanks, applied.

> diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
> @@ -1637,6 +1637,7 @@ static const struct parse_data ssid_fields[] = {
>  #ifdef CONFIG_HT_OVERRIDES
>  	{ INT_RANGE(disable_ht, 0, 1) },
>  	{ INT_RANGE(disable_ht40, -1, 1) },
> +	{ INT_RANGE(disable_sgi, -1, 1) },
>  	{ INT_RANGE(disable_max_amsdu, -1, 1) },

Though, I changed that range to 0..1 since -1 did not seem to make sense
for it. I'd assume that disable_ht40 range is also incorrect, i.e., only
the A-MSDU parameters use -1.
Ben Greear - Dec. 18, 2012, 2:59 p.m.
On 12/18/2012 04:46 AM, Jouni Malinen wrote:
> On Thu, Dec 13, 2012 at 09:30:26AM -0800, greearb@candelatech.com wrote:
>> Requires kernel patch to make the SGI-20 properly disabled...SGI-40
>> will already work since kernel 3.4 or so.
>
> Thanks, applied.
>
>> diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
>> @@ -1637,6 +1637,7 @@ static const struct parse_data ssid_fields[] = {
>>   #ifdef CONFIG_HT_OVERRIDES
>>   	{ INT_RANGE(disable_ht, 0, 1) },
>>   	{ INT_RANGE(disable_ht40, -1, 1) },
>> +	{ INT_RANGE(disable_sgi, -1, 1) },
>>   	{ INT_RANGE(disable_max_amsdu, -1, 1) },
>
> Though, I changed that range to 0..1 since -1 did not seem to make sense
> for it. I'd assume that disable_ht40 range is also incorrect, i.e., only
> the A-MSDU parameters use -1.

My intention was to have -1 mean 'don't set', ie don't set the mask for it.
0 means disabled,
1 means enabled.

I'm not sure that matters all that much, we could just as easily
default to enabled.

I'll double-check that patch when I get a chance.

Thanks,
Ben

>
Jouni Malinen - Dec. 18, 2012, 4:40 p.m.
On Tue, Dec 18, 2012 at 06:59:45AM -0800, Ben Greear wrote:
> On 12/18/2012 04:46 AM, Jouni Malinen wrote:
> > On Thu, Dec 13, 2012 at 09:30:26AM -0800, greearb@candelatech.com wrote:
> >>   	{ INT_RANGE(disable_ht40, -1, 1) },
> >> +	{ INT_RANGE(disable_sgi, -1, 1) },
> >>   	{ INT_RANGE(disable_max_amsdu, -1, 1) },
> >
> > Though, I changed that range to 0..1 since -1 did not seem to make sense
> > for it. I'd assume that disable_ht40 range is also incorrect, i.e., only
> > the A-MSDU parameters use -1.
> 
> My intention was to have -1 mean 'don't set', ie don't set the mask for it.

That's what disable_max_amsdu does. However, wpa_set_disable_ht40() and
wpa_set_disable_sgi() does not have any special case for -1 and as such,
that would be interpreted as disabled rather than don't set.

> I'm not sure that matters all that much, we could just as easily
> default to enabled.

We do exactly that.. disable_ht40/sgi are set to 0 by default which
means those are enabled. disable_ht40 could potentially be set to -1 in
the configuration and that would result in HT40 being disabled. With my
change for the disable_sgi range, this confusing value would be rejected
by configuration parser.

Patch

diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
index 48efbb0..516a2a9 100644
--- a/wpa_supplicant/config.c
+++ b/wpa_supplicant/config.c
@@ -1637,6 +1637,7 @@  static const struct parse_data ssid_fields[] = {
 #ifdef CONFIG_HT_OVERRIDES
 	{ INT_RANGE(disable_ht, 0, 1) },
 	{ INT_RANGE(disable_ht40, -1, 1) },
+	{ INT_RANGE(disable_sgi, -1, 1) },
 	{ INT_RANGE(disable_max_amsdu, -1, 1) },
 	{ INT_RANGE(ampdu_factor, -1, 3) },
 	{ INT_RANGE(ampdu_density, -1, 7) },
@@ -2037,6 +2038,7 @@  void wpa_config_set_network_defaults(struct wpa_ssid *ssid)
 #ifdef CONFIG_HT_OVERRIDES
 	ssid->disable_ht = DEFAULT_DISABLE_HT;
 	ssid->disable_ht40 = DEFAULT_DISABLE_HT40;
+	ssid->disable_sgi = DEFAULT_DISABLE_SGI;
 	ssid->disable_max_amsdu = DEFAULT_DISABLE_MAX_AMSDU;
 	ssid->ampdu_factor = DEFAULT_AMPDU_FACTOR;
 	ssid->ampdu_density = DEFAULT_AMPDU_DENSITY;
diff --git a/wpa_supplicant/config_ssid.h b/wpa_supplicant/config_ssid.h
index a9a477a..9ac67c7 100644
--- a/wpa_supplicant/config_ssid.h
+++ b/wpa_supplicant/config_ssid.h
@@ -28,6 +28,7 @@ 
 #define DEFAULT_BG_SCAN_PERIOD -1
 #define DEFAULT_DISABLE_HT 0
 #define DEFAULT_DISABLE_HT40 0
+#define DEFAULT_DISABLE_SGI 0
 #define DEFAULT_DISABLE_MAX_AMSDU -1 /* no change */
 #define DEFAULT_AMPDU_FACTOR -1 /* no change */
 #define DEFAULT_AMPDU_DENSITY -1 /* no change */
@@ -495,6 +496,14 @@  struct wpa_ssid {
 	int disable_ht40;
 
 	/**
+	 * disable_sgi - Disable SGI (Short Guard Interval) for this network
+	 *
+	 * By default, use it if it is available, but this can be configured
+	 * to 1 to have it disabled.
+	 */
+	int disable_sgi;
+
+	/**
 	 * disable_max_amsdu - Disable MAX A-MSDU
 	 *
 	 * A-MDSU will be 3839 bytes when disabled, or 7935
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 784a471..d2d88dc 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -2526,7 +2526,6 @@  static int wpa_set_ampdu_factor(struct wpa_supplicant *wpa_s,
 	return 0;
 }
 
-
 static int wpa_set_ampdu_density(struct wpa_supplicant *wpa_s,
 				 struct ieee80211_ht_capabilities *htcaps,
 				 struct ieee80211_ht_capabilities *htcaps_mask,
@@ -2573,6 +2572,27 @@  static int wpa_set_disable_ht40(struct wpa_supplicant *wpa_s,
 	return 0;
 }
 
+static int wpa_set_disable_sgi(struct wpa_supplicant *wpa_s,
+			       struct ieee80211_ht_capabilities *htcaps,
+			       struct ieee80211_ht_capabilities *htcaps_mask,
+			       int disabled)
+{
+	/* Masking these out disables SGI */
+	u16 msk = host_to_le16(HT_CAP_INFO_SHORT_GI20MHZ |
+			       HT_CAP_INFO_SHORT_GI40MHZ);
+
+	wpa_msg(wpa_s, MSG_DEBUG, "set_disable_sgi: %d", disabled);
+
+	if (disabled)
+		htcaps->ht_capabilities_info &= ~msk;
+	else
+		htcaps->ht_capabilities_info |= msk;
+
+	htcaps_mask->ht_capabilities_info |= msk;
+
+	return 0;
+}
+
 
 void wpa_supplicant_apply_ht_overrides(
 	struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid,
@@ -2596,6 +2616,7 @@  void wpa_supplicant_apply_ht_overrides(
 	wpa_set_ampdu_factor(wpa_s, htcaps, htcaps_mask, ssid->ampdu_factor);
 	wpa_set_ampdu_density(wpa_s, htcaps, htcaps_mask, ssid->ampdu_density);
 	wpa_set_disable_ht40(wpa_s, htcaps, htcaps_mask, ssid->disable_ht40);
+	wpa_set_disable_sgi(wpa_s, htcaps, htcaps_mask, ssid->disable_sgi);
 }
 
 #endif /* CONFIG_HT_OVERRIDES */