[PATCH] hostapd: take configured antenna set into consideration

[Ben Greear]

On 12/24/2015 10:41 AM, Jouni Malinen wrote:
> 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 at 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..

My patch was certainly wrong....I think this fixes it..it at least works
as expected on an ath10k NIC as far as I can tell:

diff --git a/src/ap/ieee802_11_vht.c b/src/ap/ieee802_11_vht.c
index ce7cbcf..7795f5b 100644
--- a/src/ap/ieee802_11_vht.c
+++ b/src/ap/ieee802_11_vht.c
@@ -56,19 +56,27 @@ u8 * hostapd_eid_vht_capabilities(struct hostapd_data *hapd, u8 *eid)
          * range.
          */
         if (mode->conf_tx_ant) {
+               wpa_printf(MSG_DEBUG, "configured tx-antenna: 0x%x, vht tx_map before: 0x%x",
+                          mode->conf_tx_ant, cap->vht_supported_mcs_set.tx_map);
                 for (i = 7; i > 0; i--) {
                         if (mode->conf_tx_ant & BIT(i - 1))
                                 break;
-                       cap->vht_supported_mcs_set.tx_map |= (0x3 << i);
+                       cap->vht_supported_mcs_set.tx_map |= (0x3 << ((i - 1) * 2));
                 }
+               wpa_printf(MSG_DEBUG, "configured tx-antenna: 0x%x, vht tx_map after: 0x%x",
+                          mode->conf_tx_ant, cap->vht_supported_mcs_set.tx_map);
         }

         if (mode->conf_rx_ant) {
+               wpa_printf(MSG_DEBUG, "configured rx-antenna: 0x%x, vht rx_map before: 0x%x",
+                          mode->conf_rx_ant, cap->vht_supported_mcs_set.rx_map);
                 for (i = 7; i > 0; i--) {
                         if (mode->conf_rx_ant & BIT(i - 1))
                                 break;
-                       cap->vht_supported_mcs_set.rx_map |= (0x3 << i);
+                       cap->vht_supported_mcs_set.rx_map |= (0x3 << ((i - 1) * 2));
                 }
+               wpa_printf(MSG_DEBUG, "configured rx-antenna: 0x%x, vht rx_map after: 0x%x",
+                          mode->conf_rx_ant, cap->vht_supported_mcs_set.rx_map);
         }

         pos += sizeof(*cap);


Let me know if you see any obvious problems with this patch..otherwise I'll post
it as a real patch once I get a bit of testing done on it.

>
>> 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 ..

Yes, I think you are right on this.  At least at one time, I'm certain
there were restrictions in at least ath9k, but now I see that ath9k
can support 0x2 as a value, for instance, and an arbitrary mask with
at least some chipsets.

I'm not entirely sure what hostapd should do about this since
it appears quite dependent on the low-level details of the
wifi NIC driver.

So, I think just stopping at the first set bit, as my patch did, is probably
OK, and at least it should work properly more often than the old code.

For best results, hostapd should probably have some explicit config
options to configure the MCS sets so users can make the needed configuration changes in
cases where hostapd cannot guess it properly.

Thanks,
Ben


-- 
Ben Greear <greearb at candelatech.com>
Candela Technologies Inc  http://www.candelatech.com