diff mbox

hostapd: take configured antenna set into consideration

Message ID 20151220201235.GA26143@w1.fi
State Changes Requested
Headers show

Commit Message

Jouni Malinen Dec. 20, 2015, 8:12 p.m. UTC 
On Wed, Dec 09, 2015 at 12:20:33PM -0800, greearb@candelatech.com wrote:
> If user has configured TX antennas to be less than what
> hardware advertises, the MCS reported by hardware will
> be too large.  So, remove MCS sets accordingly.

The nl80211 related changes seem mostly reasonable, but I'm not sure I
understood what exactly the HT/VHT changes were supposed to do. Why are
they different? HT uses tx_ant & rx_ant and masks values from
cap->supported_mcs_set[] until the highest 1 bit is found. VHT masks
values separately for TX and RX and does this with |= 0x3 << i. Is that
really correct? Should i be "i * 2"? Why is there difference in the
configured bitmap checks (BIT(i) vs. BIT(i - 1) and different i > 0 vs.
i >= 0 condition)?

Is it correct to find the MSB 1 from the values instead of count the
number of 1 bits (configured antennas)?

I'm attaching the cleaned up patches from my work branch. Please provide
any possible updated on top of these.

Comments

Ben Greear Dec. 20, 2015, 9:54 p.m. UTC | #1 
On 12/20/2015 12:12 PM, Jouni Malinen wrote:
> On Wed, Dec 09, 2015 at 12:20:33PM -0800, greearb@candelatech.com wrote:
>> If user has configured TX antennas to be less than what
>> hardware advertises, the MCS reported by hardware will
>> be too large.  So, remove MCS sets accordingly.
>
> The nl80211 related changes seem mostly reasonable, but I'm not sure I
> understood what exactly the HT/VHT changes were supposed to do. Why are
> they different? HT uses tx_ant & rx_ant and masks values from
> cap->supported_mcs_set[] until the highest 1 bit is found. VHT masks
> values separately for TX and RX and does this with |= 0x3 << i. Is that
> really correct? Should i be "i * 2"? Why is there difference in the
> configured bitmap checks (BIT(i) vs. BIT(i - 1) and different i > 0 vs.
> i >= 0 condition)?

There might be several ways to make the math work.  I think my math
was proper enough, but maybe though more dumb luck than theoretical correctness.

And, I'm not really sure what to do when tx-antenna doesn't match
rx-antenna.

> Is it correct to find the MSB 1 from the values instead of count the
> number of 1 bits (configured antennas)?

As far as I know, there is no valid tx/rx mask that is not contiguous
for any driver.

>
> I'm attaching the cleaned up patches from my work branch. Please provide
> any possible updated on top of these.

I'll rebase on top of your changes when I get a chance...

Thanks,
Ben
Jouni Malinen Dec. 24, 2015, 6:41 p.m. UTC | #2 
On Sun, Dec 20, 2015 at 01:54:27PM -0800, Ben Greear wrote:
> On 12/20/2015 12:12 PM, Jouni Malinen wrote:
> >On Wed, Dec 09, 2015 at 12:20:33PM -0800, greearb@candelatech.com wrote:
> >>If user has configured TX antennas to be less than what
> >>hardware advertises, the MCS reported by hardware will
> >>be too large.  So, remove MCS sets accordingly.
> >
> >The nl80211 related changes seem mostly reasonable, but I'm not sure I
> >understood what exactly the HT/VHT changes were supposed to do. Why are
> >they different? HT uses tx_ant & rx_ant and masks values from
> >cap->supported_mcs_set[] until the highest 1 bit is found. VHT masks
> >values separately for TX and RX and does this with |= 0x3 << i. Is that
> >really correct? Should i be "i * 2"? Why is there difference in the
> >configured bitmap checks (BIT(i) vs. BIT(i - 1) and different i > 0 vs.
> >i >= 0 condition)?
> 
> There might be several ways to make the math work.  I think my math
> was proper enough, but maybe though more dumb luck than theoretical correctness.

When I tested this with mac80211_hwsim and max 8 streams from the driver
configured to use only two antennas, the VHT elements did look at all
correct to me..

> And, I'm not really sure what to do when tx-antenna doesn't match
> rx-antenna.

> As far as I know, there is no valid tx/rx mask that is not contiguous
> for any driver.

Where is this documented? I could not find any real details on how the
iw "set antenna" command is supposed to be used.. That allows completely
independent bitmaps of antennas for TX and RX and there is no
requirements of being contiguous or identical between TX and RX ..
diff mbox

Patch

From 520923ac1289d37c17bcb854113c26c03e9915bc Mon Sep 17 00:00:00 2001
From: Jouni Malinen <j@w1.fi>
Date: Sun, 20 Dec 2015 21:42:46 +0200
Subject: [PATCH 3/3] tests: HT/VHT and antenna configuration

Signed-off-by: Jouni Malinen <j@w1.fi>
---
 tests/hwsim/test_ap_ht.py  | 27 +++++++++++++++++++++++++++
 tests/hwsim/test_ap_vht.py | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/tests/hwsim/test_ap_ht.py b/tests/hwsim/test_ap_ht.py
index 001e722..0922e3c 100644
--- a/tests/hwsim/test_ap_ht.py
+++ b/tests/hwsim/test_ap_ht.py
@@ -1128,3 +1128,30 @@  def test_ap_ht40_5ghz_invalid_pair(dev, apdev):
             raise Exception("AP setup failure timed out")
     finally:
         subprocess.call(['iw', 'reg', 'set', '00'])
+
+def test_ap_ht_antenna_config(dev, apdev):
+    """HT and antenna configuration"""
+    hapd = hostapd.add_ap(apdev[0]['ifname'], { "ssid": "open" })
+    phy = hapd.get_driver_status_field("phyname")
+    hapd.disable()
+
+    try:
+        subprocess.call(['iw', 'phy', phy, 'set', 'antenna', '0x1'])
+        hapd = None
+        params = { "ssid": "ht",
+                   "country_code": "FI",
+                   "hw_mode": "a",
+                   "channel": "36",
+                   "ieee80211n": "1" }
+        hapd = hostapd.add_ap(apdev[0]['ifname'], params)
+        bssid = apdev[0]['bssid']
+
+        dev[0].connect("ht", key_mgmt="NONE", scan_freq="5180")
+        hwsim_utils.test_connectivity(dev[0], hapd)
+    finally:
+        dev[0].request("DISCONNECT")
+        if hapd:
+            hapd.request("DISABLE")
+        subprocess.call(['iw', 'reg', 'set', '00'])
+        dev[0].flush_scan_cache()
+        subprocess.call(['iw', 'phy', phy, 'set', 'antenna', 'all'])
diff --git a/tests/hwsim/test_ap_vht.py b/tests/hwsim/test_ap_vht.py
index 38ecd4e..1fe539a 100644
--- a/tests/hwsim/test_ap_vht.py
+++ b/tests/hwsim/test_ap_vht.py
@@ -616,3 +616,39 @@  def test_ap_vht80_pwr_constraint(dev, apdev):
             hapd.request("DISABLE")
         subprocess.call(['iw', 'reg', 'set', '00'])
         dev[0].flush_scan_cache()
+
+def test_ap_vht80_antenna_config(dev, apdev):
+    """VHT with 80 MHz channel width and antenna configuration"""
+    hapd = hostapd.add_ap(apdev[0]['ifname'], { "ssid": "open" })
+    phy = hapd.get_driver_status_field("phyname")
+    hapd.disable()
+
+    try:
+        subprocess.call(['iw', 'phy', phy, 'set', 'antenna', '0x3'])
+        hapd = None
+        params = { "ssid": "vht",
+                   "country_code": "FI",
+                   "hw_mode": "a",
+                   "channel": "36",
+                   "ht_capab": "[HT40+]",
+                   "ieee80211n": "1",
+                   "ieee80211ac": "1",
+                   "vht_oper_chwidth": "1",
+                   "vht_oper_centr_freq_seg0_idx": "42" }
+        hapd = hostapd.add_ap(apdev[0]['ifname'], params)
+        bssid = apdev[0]['bssid']
+
+        dev[0].connect("vht", key_mgmt="NONE", scan_freq="5180")
+        hwsim_utils.test_connectivity(dev[0], hapd)
+    except Exception, e:
+        if isinstance(e, Exception) and str(e) == "AP startup failed":
+            if not vht_supported():
+                raise HwsimSkip("80 MHz channel not supported in regulatory information")
+        raise
+    finally:
+        dev[0].request("DISCONNECT")
+        if hapd:
+            hapd.request("DISABLE")
+        subprocess.call(['iw', 'reg', 'set', '00'])
+        dev[0].flush_scan_cache()
+        subprocess.call(['iw', 'phy', phy, 'set', 'antenna', 'all'])
-- 
1.9.1