2017-08-05 08:45:23

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 00/10] mac80211 patches from our internal tree 2017-08-05

From: Luca Coelho <[email protected]>

Hi,

Here are some pending mac80211 patches from our internal tree.

The "mac80211: add api to start ba session timer expired flow" patch
is needed by an iwlwifi patch that I want to send for -fixes, so it
would have to be applied to -fixes as well. Should I send a separate
patchset with these two so they can be both applied at the same time
(either in the mac80211 or in wireless-drivers tree)?

Please review.

Cheers,
Luca.

Avraham Stern (1):
ieee80211: Add WFA TPC report element OUI type

Beni Lev (1):
mac80211_hwsim: Add command to control rx status RSSI

Emmanuel Grumbach (1):
cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}

Gregory Greenman (1):
mac80211: recalculate some sta parameters after insertion

Johannes Berg (1):
mac80211: simplify and clarify IE splitting

Liad Kaufman (2):
mac80211: add MESH IE in the correct order
mac80211: extend ieee80211_ie_split to support EXTENSION

Naftali Goldstein (1):
mac80211: add api to start ba session timer expired flow

Sharon Dvir (1):
mac80211: shorten debug prints using ht_dbg() to avoid warning

Tova Mussai (1):
nl80211: return error for invalid center_freq in 40 MHz

drivers/net/wireless/mac80211_hwsim.c | 36 ++++++++++++++++++++--
include/linux/ieee80211.h | 1 +
include/net/cfg80211.h | 6 ++--
include/net/mac80211.h | 15 +++++++++
net/mac80211/agg-rx.c | 22 +++++++++++++-
net/mac80211/agg-tx.c | 4 +--
net/mac80211/mlme.c | 19 +++++-------
net/mac80211/sta_info.c | 57 ++++++++++++++++++++---------------
net/mac80211/util.c | 23 ++++++--------
net/wireless/nl80211.c | 9 ++++++
net/wireless/reg.c | 20 ++++++++++--
net/wireless/util.c | 54 ++++++++++++++++++++++++++++-----
12 files changed, 199 insertions(+), 67 deletions(-)

--
2.13.2


2017-08-05 23:14:40

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 07/10] mac80211: add api to start ba session timer expired flow

Hi Naftali,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.13-rc3 next-20170804]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Luca-Coelho/mac80211-patches-from-our-internal-tree-2017-08-05/20170806-053032
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'
>> include/net/mac80211.h:5515: warning: No description found for parameter 'bit'
>> include/net/mac80211.h:5515: warning: Excess function parameter 'tid' description in 'ieee80211_rx_ba_timer_expired'

vim +/bit +5515 include/net/mac80211.h

5501
5502 /**
5503 * ieee80211_rx_ba_timer_expired - stop a Rx BA session due to timeout
5504 *
5505 * Some device drivers do not offload AddBa/DelBa negotiation, but handle rx
5506 * buffer reording internally, and therefore also handle the session timer.
5507 *
5508 * Trigger the timeout flow, which sends a DelBa.
5509 *
5510 * @vif: &struct ieee80211_vif pointer from the add_interface callback
5511 * @addr: station mac address
5512 * @tid: the rx tid
5513 */
5514 void ieee80211_rx_ba_timer_expired(struct ieee80211_vif *vif,
> 5515 const u8 *addr, unsigned int bit);
5516

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.29 kB)
.config.gz (6.58 kB)
Download all attachments

2017-08-06 07:15:34

by Luca Coelho

[permalink] [raw]
Subject: [PATCH v2] mac80211: add api to start ba session timer expired flow

From: Naftali Goldstein <[email protected]>

Some drivers handle rx buffer reordering internally (and by extension
handle also the rx ba session timer internally), but do not ofload the
addba/delba negotiation.
Add an api for these drivers to properly tear-down the ba session,
including sending a delba.

Signed-off-by: Naftali Goldstein <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---

In v2:

* fix htmldocs warning, reported by 0-day

include/net/mac80211.h | 15 +++++++++++++++
net/mac80211/agg-rx.c | 22 +++++++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index b2b5419467cc..f8149ca192b4 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5499,6 +5499,21 @@ static inline void ieee80211_stop_rx_ba_session_offl(struct ieee80211_vif *vif,
ieee80211_manage_rx_ba_offl(vif, addr, tid + IEEE80211_NUM_TIDS);
}

+/**
+ * ieee80211_rx_ba_timer_expired - stop a Rx BA session due to timeout
+ *
+ * Some device drivers do not offload AddBa/DelBa negotiation, but handle rx
+ * buffer reording internally, and therefore also handle the session timer.
+ *
+ * Trigger the timeout flow, which sends a DelBa.
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback
+ * @addr: station mac address
+ * @tid: the rx tid
+ */
+void ieee80211_rx_ba_timer_expired(struct ieee80211_vif *vif,
+ const u8 *addr, unsigned int tid);
+
/* Rate control API */

/**
diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 8708cbe8af5b..2b36eff5d97e 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -7,7 +7,7 @@
* Copyright 2006-2007 Jiri Benc <[email protected]>
* Copyright 2007, Michael Wu <[email protected]>
* Copyright 2007-2010, Intel Corporation
- * Copyright(c) 2015 Intel Deutschland GmbH
+ * Copyright(c) 2015-2017 Intel Deutschland GmbH
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -466,3 +466,23 @@ void ieee80211_manage_rx_ba_offl(struct ieee80211_vif *vif,
rcu_read_unlock();
}
EXPORT_SYMBOL(ieee80211_manage_rx_ba_offl);
+
+void ieee80211_rx_ba_timer_expired(struct ieee80211_vif *vif,
+ const u8 *addr, unsigned int tid)
+{
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+ struct ieee80211_local *local = sdata->local;
+ struct sta_info *sta;
+
+ rcu_read_lock();
+ sta = sta_info_get_bss(sdata, addr);
+ if (!sta)
+ goto unlock;
+
+ set_bit(tid, sta->ampdu_mlme.tid_rx_timer_expired);
+ ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work);
+
+ unlock:
+ rcu_read_unlock();
+}
+EXPORT_SYMBOL(ieee80211_rx_ba_timer_expired);
--
2.13.2

2017-08-05 08:45:23

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 05/10] ieee80211: Add WFA TPC report element OUI type

From: Avraham Stern <[email protected]>

Signed-off-by: Avraham Stern <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
include/linux/ieee80211.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 34e1bcd2d7ff..6572c2ca1261 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2412,6 +2412,7 @@ enum ieee80211_sa_query_action {
#define WLAN_OUI_TYPE_MICROSOFT_WPA 1
#define WLAN_OUI_TYPE_MICROSOFT_WMM 2
#define WLAN_OUI_TYPE_MICROSOFT_WPS 4
+#define WLAN_OUI_TYPE_MICROSOFT_TPC 8

/*
* WMM/802.11e Tspec Element
--
2.13.2

2017-08-05 08:45:23

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 01/10] mac80211: add MESH IE in the correct order

From: Liad Kaufman <[email protected]>

VHT MESH support was added, but the order of the IEs
wasn't enforced. Fix that.

Signed-off-by: Liad Kaufman <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
net/mac80211/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 259698de569f..6aef6793d052 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1436,7 +1436,7 @@ static int ieee80211_build_preq_ies_band(struct ieee80211_local *local,
WLAN_EID_SSID_LIST,
WLAN_EID_CHANNEL_USAGE,
WLAN_EID_INTERWORKING,
- /* mesh ID can't happen here */
+ WLAN_EID_MESH_ID,
/* 60 GHz can't happen here right now */
};
noffset = ieee80211_ie_split(ie, ie_len,
--
2.13.2

2017-08-07 13:05:40

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 00/10] mac80211 patches from our internal tree 2017-08-05

Luca Coelho <[email protected]> writes:

> On Mon, 2017-08-07 at 12:06 +0300, Kalle Valo wrote:
>> Luca Coelho <[email protected]> writes:
>>
>> > Here are some pending mac80211 patches from our internal tree.
>> >
>> > The "mac80211: add api to start ba session timer expired flow" patch
>> > is needed by an iwlwifi patch that I want to send for -fixes, so it
>> > would have to be applied to -fixes as well.
>>
>> We are getting to the later stages of the release cycle and I'm raising
>> the bar for wireless-drivers even higher. How serious iwlwifi bug is
>> that fixing?
>
> The problem is a bad degradation in throughput with our new 9000 family
> of devices because when aggregation times out we stop the aggregation
> internally in the firmware but don't send a delba. Then on the AP side,
> also with our driver, we don't handle the timeout as we should, so
> aggregations the devices get out of sync and BA sessions are not
> possible anymore, limiting our throughput to ~30Mbps, in some specific
> internal tests.

Ok.

>> > Should I send a separate patchset with these two so they can be both
>> > applied at the same time (either in the mac80211 or in
>> > wireless-drivers tree)?
>>
>> Now that Johannes is away and I'm taking any urgent mac80211 patches, I
>> think the best approach is that you include mac80211 patch in the same
>> patchset as the iwlwifi patches destined for wireless-drivers.
>
> Okay, if you think the (one-liner) iwlwifi driver fix is -rc'able, I'll
> resend this in a patchset including both changes.

If the iwlwifi patch is a oneliner it doesn't sound too bad, but I
reserve the right to change my mind :)

But try to submit the pull request in the next few days so that I can
submit the patches forward by end of this week. And I think it's easiest
that you apply the mac80211 patch directly to your tree and I just pull
from you.

--
Kalle Valo

2017-08-05 08:45:37

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 10/10] nl80211: return error for invalid center_freq in 40 MHz

From: Tova Mussai <[email protected]>

When NL80211_ATTR_WIPHY_CHANNEL_TYPE is given, nl80211 would parse the
channel definition the old way, discarding NL80211_ATTR_CENTER_FREQ1,
NL80211_ATTR_CENTER_FREQ2 etc. However, it is possible that user space
added both NL80211_ATTR_WIPHY_CHANNEL_TYPE and NL80211_ATTR_CENTER_FREQ1
or NL80211_ATTR_CENTER_FREQ2 assuming that all settings would be honored.

In such a case, validate that NL80211_ATTR_CENTER_FREQ1 and
NL80211_ATTR_CENTER_FREQ2 values match the channel configuration,
as otherwise user space would assume that the desired configuration was
applied.

For example, when trying to start ap with
NL80211_ATTR_WIPHY_CHANNEL_TYPE = NL80211_CHAN_HT40MINUS,
NL80211_ATTR_WIPHY_FREQ = 5180 and NL80211_ATTR_CENTER_FREQ1 = 5250
without this fix, the ap will start on channel 36 (center_freq1 will be
corrected to 5180). With this fix, we will throw an error instead.

Signed-off-by: Tova Mussai <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
net/wireless/nl80211.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 45ba3d0872cc..edf484fe1f80 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2122,6 +2122,15 @@ static int nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
case NL80211_CHAN_HT40MINUS:
cfg80211_chandef_create(chandef, chandef->chan,
chantype);
+ /* user input for center_freq is incorrect */
+ if (info->attrs[NL80211_ATTR_CENTER_FREQ1] &&
+ chandef->center_freq1 != nla_get_u32(
+ info->attrs[NL80211_ATTR_CENTER_FREQ1]))
+ return -EINVAL;
+ /* center_freq2 must be zero */
+ if (info->attrs[NL80211_ATTR_CENTER_FREQ2] &&
+ nla_get_u32(info->attrs[NL80211_ATTR_CENTER_FREQ2]))
+ return -EINVAL;
break;
default:
return -EINVAL;
--
2.13.2

2017-08-05 08:45:22

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 03/10] mac80211: extend ieee80211_ie_split to support EXTENSION

From: Liad Kaufman <[email protected]>

Current ieee80211_ie_split() implementation doesn't
account for elements that are sub-elements of the
EXTENSION IE. To extend support to these IEs as well,
treat the WLAN_EID_EXTENSION ids in the %ids array
as indicating that the next id in the array is a
sub-element of the EXTENSION IE.

Signed-off-by: Liad Kaufman <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
include/net/cfg80211.h | 6 ++++--
net/wireless/util.c | 54 ++++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f12fa5245a45..aa9d993e519a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5934,7 +5934,8 @@ int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
* @ies: the IE buffer
* @ielen: the length of the IE buffer
* @ids: an array with element IDs that are allowed before
- * the split
+ * the split. A WLAN_EID_EXTENSION value means that the next
+ * EID in the list is a sub-element of the EXTENSION IE.
* @n_ids: the size of the element ID array
* @after_ric: array IE types that come after the RIC element
* @n_after_ric: size of the @after_ric array
@@ -5965,7 +5966,8 @@ size_t ieee80211_ie_split_ric(const u8 *ies, size_t ielen,
* @ies: the IE buffer
* @ielen: the length of the IE buffer
* @ids: an array with element IDs that are allowed before
- * the split
+ * the split. A WLAN_EID_EXTENSION value means that the next
+ * EID in the list is a sub-element of the EXTENSION IE.
* @n_ids: the size of the element ID array
* @offset: offset where to start splitting in the buffer
*
diff --git a/net/wireless/util.c b/net/wireless/util.c
index bcb1284c3415..4aab793c2f00 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1367,13 +1367,29 @@ int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
}
EXPORT_SYMBOL(cfg80211_get_p2p_attr);

-static bool ieee80211_id_in_list(const u8 *ids, int n_ids, u8 id)
+static bool ieee80211_id_in_list(const u8 *ids, int n_ids, u8 id, bool id_ext)
{
int i;

- for (i = 0; i < n_ids; i++)
- if (ids[i] == id)
+ /* Make sure array values are legal */
+ if (WARN_ON(ids[n_ids - 1] == WLAN_EID_EXTENSION))
+ return false;
+
+ i = 0;
+ while (i < n_ids) {
+ if (ids[i] == WLAN_EID_EXTENSION) {
+ if (id_ext && (ids[i + 1] == id))
+ return true;
+
+ i += 2;
+ continue;
+ }
+
+ if (ids[i] == id && !id_ext)
return true;
+
+ i++;
+ }
return false;
}

@@ -1403,14 +1419,36 @@ size_t ieee80211_ie_split_ric(const u8 *ies, size_t ielen,
{
size_t pos = offset;

- while (pos < ielen && ieee80211_id_in_list(ids, n_ids, ies[pos])) {
+ while (pos < ielen) {
+ u8 ext = 0;
+
+ if (ies[pos] == WLAN_EID_EXTENSION)
+ ext = 2;
+ if ((pos + ext) >= ielen)
+ break;
+
+ if (!ieee80211_id_in_list(ids, n_ids, ies[pos + ext],
+ ies[pos] == WLAN_EID_EXTENSION))
+ break;
+
if (ies[pos] == WLAN_EID_RIC_DATA && n_after_ric) {
pos = skip_ie(ies, ielen, pos);

- while (pos < ielen &&
- !ieee80211_id_in_list(after_ric, n_after_ric,
- ies[pos]))
- pos = skip_ie(ies, ielen, pos);
+ while (pos < ielen) {
+ if (ies[pos] == WLAN_EID_EXTENSION)
+ ext = 2;
+ else
+ ext = 0;
+
+ if ((pos + ext) >= ielen)
+ break;
+
+ if (!ieee80211_id_in_list(after_ric,
+ n_after_ric,
+ ies[pos + ext],
+ ext == 2))
+ pos = skip_ie(ies, ielen, pos);
+ }
} else {
pos = skip_ie(ies, ielen, pos);
}
--
2.13.2

2017-08-05 08:45:26

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 08/10] mac80211: recalculate some sta parameters after insertion

From: Gregory Greenman <[email protected]>

Sometimes a station is added already in ASSOC state. For example,
in AP mode, when a client station didn't get assoc resp and sends
an assoc req again. If a station is inserted when its state is ASSOC
or higher, the min chandef and allow_p2p_go_ps should be recalculated
again after the insertion.

Before this patch the recalculation happened only in sta_info_move_state
which occurs before the insertion of the sta and thus even though
it calls ieee80211_recalc_min_chandef/_p2p_go_ps_allowed functions,
since sdata->local->sta_list is still empty at this point, it doesn't do
anything.

Signed-off-by: Gregory Greenman <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
net/mac80211/sta_info.c | 57 +++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 69615016d5bf..ffcd25c4908c 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -515,6 +515,31 @@ static int sta_info_insert_drv_state(struct ieee80211_local *local,
return err;
}

+static void
+ieee80211_recalc_p2p_go_ps_allowed(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_local *local = sdata->local;
+ bool allow_p2p_go_ps = sdata->vif.p2p;
+ struct sta_info *sta;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ if (sdata != sta->sdata ||
+ !test_sta_flag(sta, WLAN_STA_ASSOC))
+ continue;
+ if (!sta->sta.support_p2p_ps) {
+ allow_p2p_go_ps = false;
+ break;
+ }
+ }
+ rcu_read_unlock();
+
+ if (allow_p2p_go_ps != sdata->vif.bss_conf.allow_p2p_go_ps) {
+ sdata->vif.bss_conf.allow_p2p_go_ps = allow_p2p_go_ps;
+ ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_P2P_PS);
+ }
+}
+
/*
* should be called with sta_mtx locked
* this function replaces the mutex lock
@@ -561,6 +586,13 @@ static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
goto out_remove;

set_sta_flag(sta, WLAN_STA_INSERTED);
+
+ if (sta->sta_state >= IEEE80211_STA_ASSOC) {
+ ieee80211_recalc_min_chandef(sta->sdata);
+ if (!sta->sta.support_p2p_ps)
+ ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
+ }
+
/* accept BA sessions now */
clear_sta_flag(sta, WLAN_STA_BLOCK_BA);

@@ -1788,31 +1820,6 @@ void ieee80211_sta_set_buffered(struct ieee80211_sta *pubsta,
}
EXPORT_SYMBOL(ieee80211_sta_set_buffered);

-static void
-ieee80211_recalc_p2p_go_ps_allowed(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_local *local = sdata->local;
- bool allow_p2p_go_ps = sdata->vif.p2p;
- struct sta_info *sta;
-
- rcu_read_lock();
- list_for_each_entry_rcu(sta, &local->sta_list, list) {
- if (sdata != sta->sdata ||
- !test_sta_flag(sta, WLAN_STA_ASSOC))
- continue;
- if (!sta->sta.support_p2p_ps) {
- allow_p2p_go_ps = false;
- break;
- }
- }
- rcu_read_unlock();
-
- if (allow_p2p_go_ps != sdata->vif.bss_conf.allow_p2p_go_ps) {
- sdata->vif.bss_conf.allow_p2p_go_ps = allow_p2p_go_ps;
- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_P2P_PS);
- }
-}
-
int sta_info_move_state(struct sta_info *sta,
enum ieee80211_sta_state new_state)
{
--
2.13.2

2017-08-07 09:06:48

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 00/10] mac80211 patches from our internal tree 2017-08-05

Luca Coelho <[email protected]> writes:

> Here are some pending mac80211 patches from our internal tree.
>
> The "mac80211: add api to start ba session timer expired flow" patch
> is needed by an iwlwifi patch that I want to send for -fixes, so it
> would have to be applied to -fixes as well.

We are getting to the later stages of the release cycle and I'm raising
the bar for wireless-drivers even higher. How serious iwlwifi bug is
that fixing?

> Should I send a separate patchset with these two so they can be both
> applied at the same time (either in the mac80211 or in
> wireless-drivers tree)?

Now that Johannes is away and I'm taking any urgent mac80211 patches, I
think the best approach is that you include mac80211 patch in the same
patchset as the iwlwifi patches destined for wireless-drivers.

--
Kalle Valo

2017-08-07 09:31:47

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 00/10] mac80211 patches from our internal tree 2017-08-05

On Mon, 2017-08-07 at 12:06 +0300, Kalle Valo wrote:
> Luca Coelho <[email protected]> writes:
>
> > Here are some pending mac80211 patches from our internal tree.
> >
> > The "mac80211: add api to start ba session timer expired flow" patch
> > is needed by an iwlwifi patch that I want to send for -fixes, so it
> > would have to be applied to -fixes as well.
>
> We are getting to the later stages of the release cycle and I'm raising
> the bar for wireless-drivers even higher. How serious iwlwifi bug is
> that fixing?

The problem is a bad degradation in throughput with our new 9000 family
of devices because when aggregation times out we stop the aggregation
internally in the firmware but don't send a delba. Then on the AP side,
also with our driver, we don't handle the timeout as we should, so
aggregations the devices get out of sync and BA sessions are not
possible anymore, limiting our throughput to ~30Mbps, in some specific
internal tests.


> > Should I send a separate patchset with these two so they can be both
> > applied at the same time (either in the mac80211 or in
> > wireless-drivers tree)?
>
> Now that Johannes is away and I'm taking any urgent mac80211 patches, I
> think the best approach is that you include mac80211 patch in the same
> patchset as the iwlwifi patches destined for wireless-drivers.

Okay, if you think the (one-liner) iwlwifi driver fix is -rc'able, I'll
resend this in a patchset including both changes.


--
Cheers,
Luca.

2017-08-05 08:45:23

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 02/10] mac80211: shorten debug prints using ht_dbg() to avoid warning

From: Sharon Dvir <[email protected]>

Invoking ht_dbg() with too long of a string will print a warning.
Shorten the messages while retaining the printed patameters.

Signed-off-by: Sharon Dvir <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
net/mac80211/agg-tx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index cbd48762256c..420486b5a1d9 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -436,7 +436,7 @@ static void sta_addba_resp_timer_expired(unsigned long data)
test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state)) {
rcu_read_unlock();
ht_dbg(sta->sdata,
- "timer expired on %pM tid %d but we are not (or no longer) expecting addBA response there\n",
+ "timer expired on %pM tid %d not expecting addBA response\n",
sta->sta.addr, tid);
return;
}
@@ -639,7 +639,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
time_before(jiffies, sta->ampdu_mlme.last_addba_req_time[tid] +
HT_AGG_RETRIES_PERIOD)) {
ht_dbg(sdata,
- "BA request denied - waiting a grace period after %d failed requests on %pM tid %u\n",
+ "BA request denied - %d failed requests on %pM tid %u\n",
sta->ampdu_mlme.addba_req_num[tid], sta->sta.addr, tid);
ret = -EBUSY;
goto err_unlock_sta;
--
2.13.2

2017-08-05 08:45:23

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 04/10] mac80211: simplify and clarify IE splitting

From: Johannes Berg <[email protected]>

There's no need to split off IEs from the ones obtained
from userspace, if they were already split off, so for
example IEs that went before HT don't have to be listed
again to go before VHT. Simplify the code here so it's
clearer.

While at it, also clarify the comments regarding the DMG
(60 GHz) elements.

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/mlme.c | 19 +++++++------------
net/mac80211/util.c | 21 +++++++++------------
2 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 7be7917e1541..5bb570643494 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -780,11 +780,12 @@ static void ieee80211_send_assoc(struct ieee80211_sub_if_data *sdata)
WLAN_EID_SUPPORTED_REGULATORY_CLASSES,
WLAN_EID_HT_CAPABILITY,
WLAN_EID_BSS_COEX_2040,
+ /* luckily this is almost always there */
WLAN_EID_EXT_CAPABILITY,
WLAN_EID_QOS_TRAFFIC_CAPA,
WLAN_EID_TIM_BCAST_REQ,
WLAN_EID_INTERWORKING,
- /* 60GHz doesn't happen right now */
+ /* 60 GHz (Multi-band, DMG, MMS) can't happen */
WLAN_EID_VHT_CAPABILITY,
WLAN_EID_OPMODE_NOTIF,
};
@@ -812,22 +813,16 @@ static void ieee80211_send_assoc(struct ieee80211_sub_if_data *sdata)
/* if present, add any custom IEs that go before VHT */
if (assoc_data->ie_len) {
static const u8 before_vht[] = {
- WLAN_EID_SSID,
- WLAN_EID_SUPP_RATES,
- WLAN_EID_EXT_SUPP_RATES,
- WLAN_EID_PWR_CAPABILITY,
- WLAN_EID_SUPPORTED_CHANNELS,
- WLAN_EID_RSN,
- WLAN_EID_QOS_CAPA,
- WLAN_EID_RRM_ENABLED_CAPABILITIES,
- WLAN_EID_MOBILITY_DOMAIN,
- WLAN_EID_SUPPORTED_REGULATORY_CLASSES,
- WLAN_EID_HT_CAPABILITY,
+ /*
+ * no need to list the ones split off before HT
+ * or generated here
+ */
WLAN_EID_BSS_COEX_2040,
WLAN_EID_EXT_CAPABILITY,
WLAN_EID_QOS_TRAFFIC_CAPA,
WLAN_EID_TIM_BCAST_REQ,
WLAN_EID_INTERWORKING,
+ /* 60 GHz (Multi-band, DMG, MMS) can't happen */
};

/* RIC already taken above, so no need to handle here anymore */
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 6aef6793d052..bfecc3e86318 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1392,10 +1392,10 @@ static int ieee80211_build_preq_ies_band(struct ieee80211_local *local,
/* insert custom IEs that go before HT */
if (ie && ie_len) {
static const u8 before_ht[] = {
- WLAN_EID_SSID,
- WLAN_EID_SUPP_RATES,
- WLAN_EID_REQUEST,
- WLAN_EID_EXT_SUPP_RATES,
+ /*
+ * no need to list the ones split off already
+ * (or generated here)
+ */
WLAN_EID_DS_PARAMS,
WLAN_EID_SUPPORTED_REGULATORY_CLASSES,
};
@@ -1424,20 +1424,17 @@ static int ieee80211_build_preq_ies_band(struct ieee80211_local *local,
/* insert custom IEs that go before VHT */
if (ie && ie_len) {
static const u8 before_vht[] = {
- WLAN_EID_SSID,
- WLAN_EID_SUPP_RATES,
- WLAN_EID_REQUEST,
- WLAN_EID_EXT_SUPP_RATES,
- WLAN_EID_DS_PARAMS,
- WLAN_EID_SUPPORTED_REGULATORY_CLASSES,
- WLAN_EID_HT_CAPABILITY,
+ /*
+ * no need to list the ones split off already
+ * (or generated here)
+ */
WLAN_EID_BSS_COEX_2040,
WLAN_EID_EXT_CAPABILITY,
WLAN_EID_SSID_LIST,
WLAN_EID_CHANNEL_USAGE,
WLAN_EID_INTERWORKING,
WLAN_EID_MESH_ID,
- /* 60 GHz can't happen here right now */
+ /* 60 GHz (Multi-band, DMG, MMS) can't happen */
};
noffset = ieee80211_ie_split(ie, ie_len,
before_vht, ARRAY_SIZE(before_vht),
--
2.13.2

2017-08-05 08:45:24

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 06/10] cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}

From: Emmanuel Grumbach <[email protected]>

Honor the NL80211_RRF_NO_HT40{MINUS,PLUS} flags in
reg_process_ht_flags_channel. Not doing so leads can lead
to a firmware assert in iwlwifi for example.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=195299

Fixes: b0d7aa59592b ("cfg80211: allow wiphy specific regdomain management")
Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
net/wireless/reg.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5fae296a6a58..29879440626e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -4,6 +4,7 @@
* Copyright 2007 Johannes Berg <[email protected]>
* Copyright 2008-2011 Luis R. Rodriguez <[email protected]>
* Copyright 2013-2014 Intel Mobile Communications GmbH
+ * Copyright 2017 Intel Deutschland GmbH
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -1483,7 +1484,9 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
{
struct ieee80211_supported_band *sband = wiphy->bands[channel->band];
struct ieee80211_channel *channel_before = NULL, *channel_after = NULL;
+ const struct ieee80211_regdomain *regd;
unsigned int i;
+ u32 flags;

if (!is_ht40_allowed(channel)) {
channel->flags |= IEEE80211_CHAN_NO_HT40;
@@ -1503,17 +1506,30 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
channel_after = c;
}

+ regd = get_wiphy_regdom(wiphy);
+ if (regd) {
+ const struct ieee80211_reg_rule *reg_rule =
+ freq_reg_info_regd(MHZ_TO_KHZ(channel->center_freq),
+ regd, MHZ_TO_KHZ(20));
+
+ flags = reg_rule->flags;
+ } else {
+ flags = 0;
+ }
+
/*
* Please note that this assumes target bandwidth is 20 MHz,
* if that ever changes we also need to change the below logic
* to include that as well.
*/
- if (!is_ht40_allowed(channel_before))
+ if (!is_ht40_allowed(channel_before) ||
+ flags & NL80211_RRF_NO_HT40MINUS)
channel->flags |= IEEE80211_CHAN_NO_HT40MINUS;
else
channel->flags &= ~IEEE80211_CHAN_NO_HT40MINUS;

- if (!is_ht40_allowed(channel_after))
+ if (!is_ht40_allowed(channel_after) ||
+ flags & NL80211_RRF_NO_HT40PLUS)
channel->flags |= IEEE80211_CHAN_NO_HT40PLUS;
else
channel->flags &= ~IEEE80211_CHAN_NO_HT40PLUS;
--
2.13.2

2017-08-05 08:45:27

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 09/10] mac80211_hwsim: Add command to control rx status RSSI

From: Beni Lev <[email protected]>

Set the base RSSI of a TX frame. The final RSSI of the frame will be
the base RSSI + the radio's TX power

Signed-off-by: Beni Lev <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 36 +++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 6467ffac9811..7c3811f0c730 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -368,6 +368,8 @@ static const struct ieee80211_rate hwsim_rates[] = {
{ .bitrate = 540 }
};

+#define DEFAULT_RX_RSSI -50
+
#define OUI_QCA 0x001374
#define QCA_NL80211_SUBCMD_TEST 1
enum qca_nl80211_vendor_subcmds {
@@ -571,6 +573,9 @@ struct mac80211_hwsim_data {
u64 rx_bytes;
u64 tx_dropped;
u64 tx_failed;
+
+ /* RSSI in rx status of the receiver */
+ int rx_rssi;
};


@@ -783,6 +788,29 @@ DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_group,
hwsim_fops_group_read, hwsim_fops_group_write,
"%llx\n");

+static int hwsim_fops_rx_rssi_read(void *dat, u64 *val)
+{
+ struct mac80211_hwsim_data *data = dat;
+ *val = data->rx_rssi;
+ return 0;
+}
+
+static int hwsim_fops_rx_rssi_write(void *dat, u64 val)
+{
+ struct mac80211_hwsim_data *data = dat;
+ int rssi = (int)val;
+
+ if (rssi >= 0 || rssi < -100)
+ return -EINVAL;
+
+ data->rx_rssi = rssi;
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_rx_rssi,
+ hwsim_fops_rx_rssi_read, hwsim_fops_rx_rssi_write,
+ "%lld\n");
+
static netdev_tx_t hwsim_mon_xmit(struct sk_buff *skb,
struct net_device *dev)
{
@@ -1210,8 +1238,8 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
rx_status.bw = RATE_INFO_BW_20;
if (info->control.rates[0].flags & IEEE80211_TX_RC_SHORT_GI)
rx_status.enc_flags |= RX_ENC_FLAG_SHORT_GI;
- /* TODO: simulate real signal strength (and optional packet loss) */
- rx_status.signal = -50;
+ /* TODO: simulate optional packet loss */
+ rx_status.signal = data->rx_rssi;
if (info->control.vif)
rx_status.signal += info->control.vif->bss_conf.txpower;

@@ -2554,6 +2582,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
hw->wiphy->n_iface_combinations = ARRAY_SIZE(hwsim_if_comb);
}

+ data->rx_rssi = DEFAULT_RX_RSSI;
+
INIT_DELAYED_WORK(&data->roc_start, hw_roc_start);
INIT_DELAYED_WORK(&data->roc_done, hw_roc_done);
INIT_DELAYED_WORK(&data->hw_scan, hw_scan_work);
@@ -2715,6 +2745,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
debugfs_create_file("ps", 0666, data->debugfs, data, &hwsim_fops_ps);
debugfs_create_file("group", 0666, data->debugfs, data,
&hwsim_fops_group);
+ debugfs_create_file("rx_rssi", 0666, data->debugfs, data,
+ &hwsim_fops_rx_rssi);
if (!data->use_chanctx)
debugfs_create_file("dfs_simulate_radar", 0222,
data->debugfs,
--
2.13.2

2017-08-05 08:45:25

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 07/10] mac80211: add api to start ba session timer expired flow

From: Naftali Goldstein <[email protected]>

Some drivers handle rx buffer reordering internally (and by extension
handle also the rx ba session timer internally), but do not ofload the
addba/delba negotiation.
Add an api for these drivers to properly tear-down the ba session,
including sending a delba.

Signed-off-by: Naftali Goldstein <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
include/net/mac80211.h | 15 +++++++++++++++
net/mac80211/agg-rx.c | 22 +++++++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index b2b5419467cc..64f5abb02030 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5499,6 +5499,21 @@ static inline void ieee80211_stop_rx_ba_session_offl(struct ieee80211_vif *vif,
ieee80211_manage_rx_ba_offl(vif, addr, tid + IEEE80211_NUM_TIDS);
}

+/**
+ * ieee80211_rx_ba_timer_expired - stop a Rx BA session due to timeout
+ *
+ * Some device drivers do not offload AddBa/DelBa negotiation, but handle rx
+ * buffer reording internally, and therefore also handle the session timer.
+ *
+ * Trigger the timeout flow, which sends a DelBa.
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback
+ * @addr: station mac address
+ * @tid: the rx tid
+ */
+void ieee80211_rx_ba_timer_expired(struct ieee80211_vif *vif,
+ const u8 *addr, unsigned int bit);
+
/* Rate control API */

/**
diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 8708cbe8af5b..34599bd38991 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -7,7 +7,7 @@
* Copyright 2006-2007 Jiri Benc <[email protected]>
* Copyright 2007, Michael Wu <[email protected]>
* Copyright 2007-2010, Intel Corporation
- * Copyright(c) 2015 Intel Deutschland GmbH
+ * Copyright(c) 2015-2017 Intel Deutschland GmbH
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -466,3 +466,23 @@ void ieee80211_manage_rx_ba_offl(struct ieee80211_vif *vif,
rcu_read_unlock();
}
EXPORT_SYMBOL(ieee80211_manage_rx_ba_offl);
+
+void ieee80211_rx_ba_timer_expired(struct ieee80211_vif *vif,
+ const u8 *addr, unsigned int bit)
+{
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+ struct ieee80211_local *local = sdata->local;
+ struct sta_info *sta;
+
+ rcu_read_lock();
+ sta = sta_info_get_bss(sdata, addr);
+ if (!sta)
+ goto unlock;
+
+ set_bit(bit, sta->ampdu_mlme.tid_rx_timer_expired);
+ ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work);
+
+ unlock:
+ rcu_read_unlock();
+}
+EXPORT_SYMBOL(ieee80211_rx_ba_timer_expired);
--
2.13.2

2017-09-05 16:49:41

by Grumbach, Emmanuel

[permalink] [raw]
Subject: Re: [PATCH 06/10] cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}

T24gVHVlLCAyMDE3LTA5LTA1IGF0IDE2OjMwICswMjAwLCBKb2hhbm5lcyBCZXJnIHdyb3RlOg0K
PiBPbiBTYXQsIDIwMTctMDgtMDUgYXQgMTE6NDQgKzAzMDAsIEx1Y2EgQ29lbGhvIHdyb3RlOg0K
PiANCj4gPiArCXJlZ2QgPSBnZXRfd2lwaHlfcmVnZG9tKHdpcGh5KTsNCj4gPiArCWlmIChyZWdk
KSB7DQo+ID4gKwkJY29uc3Qgc3RydWN0IGllZWU4MDIxMV9yZWdfcnVsZSAqcmVnX3J1bGUgPQ0K
PiA+ICsJCQlmcmVxX3JlZ19pbmZvX3JlZ2QoTUhaX1RPX0tIWihjaGFubmVsLQ0KPiA+ID4gY2Vu
dGVyX2ZyZXEpLA0KPiA+IA0KPiA+ICsJCQkJCcKgwqDCoHJlZ2QsIE1IWl9UT19LSFooMjApKTsN
Cj4gPiANCj4gDQo+IFRoaXMgY291bGQgcmV0dXJuIGFuIGVycm9yLCBob3cgY2FuIHlvdSBiZSBz
dXJlIGl0IGRvZXNuJ3Q/DQo+IA0KDQpIbS4uLiBzbyBJIGd1ZXNzIEkgY291bGQgY2hlY2sgdGhh
dCBpdCBkaWRuJ3QgcmV0dXJuIGFueSBlcnJvciBhbmQgaWYNCml0IGRpZCwgdGhlbiBmbGFncyA9
IDA/DQoNClNvbWV0aGluZyBsaWtlIHRoaXM/IChvbiB0b3Agb2YgdGhpcyBwYXRjaCk6DQoNCkBA
IC0xNTA2LDE1ICsxNTA2LDE1IEBAIHN0YXRpYyB2b2lkIHJlZ19wcm9jZXNzX2h0X2ZsYWdzX2No
YW5uZWwoc3RydWN0IHdpcGh5ICp3aXBoeSwNCiAgICAgICAgICAgICAgICAgICAgICAgIGNoYW5u
ZWxfYWZ0ZXIgPSBjOw0KICAgICAgICB9DQogDQorICAgICAgIGZsYWdzID0gMDsNCiAgICAgICAg
cmVnZCA9IGdldF93aXBoeV9yZWdkb20od2lwaHkpOw0KICAgICAgICBpZiAocmVnZCkgew0KICAg
ICAgICAgICAgICAgIGNvbnN0IHN0cnVjdCBpZWVlODAyMTFfcmVnX3J1bGUgKnJlZ19ydWxlID0N
CiAgICAgICAgICAgICAgICAgICAgICAgIGZyZXFfcmVnX2luZm9fcmVnZChNSFpfVE9fS0haKGNo
YW5uZWwtPmNlbnRlcl9mcmVxKSwNCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICByZWdkLCBNSFpfVE9fS0haKDIwKSk7DQogDQotICAgICAgICAgICAgICAgZmxhZ3Mg
PSByZWdfcnVsZS0+ZmxhZ3M7DQotICAgICAgIH0gZWxzZSB7DQotICAgICAgICAgICAgICAgZmxh
Z3MgPSAwOw0KKyAgICAgICAgICAgICAgIGlmICghSVNfRVJSKHJlZ19ydWxlKSkNCisgICAgICAg
ICAgICAgICAgICAgICAgIGZsYWdzID0gcmVnX3J1bGUtPmZsYWdzOw0KICAgICAgICB9DQogDQog
ICAgICAgIC8q

2017-09-06 10:33:34

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 09/10] mac80211_hwsim: Add command to control rx status RSSI

On Sat, 2017-08-05 at 11:44 +0300, Luca Coelho wrote:
>
> +static int hwsim_fops_rx_rssi_write(void *dat, u64 val)
> +{
> + struct mac80211_hwsim_data *data = dat;
> + int rssi = (int)val;

That cast is wrong - you should go to s64, do the range check, and only
then restrict to int. Basically, just do

s64 rssi = val;

in this line instead.


> + debugfs_create_file("rx_rssi", 0666, data->debugfs, data,
> +     &hwsim_fops_rx_rssi);

I also can't say I'm really happy with this, since there's no such
thing as an "RX RSSI".

Also, wmediumd already allows changing this - I'd rather people start
using the infrastructure better than papering over more.

IOW - do we really need this? Why can't you do the (simple) wmediumd
setup? There's even a hwsim test in wpa_s with wmediumd, it's not all
that difficult.

johannes

2017-09-06 07:29:43

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 06/10] cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}

On Tue, 2017-09-05 at 16:49 +0000, Grumbach, Emmanuel wrote:
> On Tue, 2017-09-05 at 16:30 +0200, Johannes Berg wrote:
> > On Sat, 2017-08-05 at 11:44 +0300, Luca Coelho wrote:
> >
> > > + regd = get_wiphy_regdom(wiphy);
> > > + if (regd) {
> > > + const struct ieee80211_reg_rule *reg_rule =
> > > + freq_reg_info_regd(MHZ_TO_KHZ(channel-
> > > > center_freq),
> > >
> > > +    regd,
> > > MHZ_TO_KHZ(20));
> > >
> >
> > This could return an error, how can you be sure it doesn't?
> >
>
> Hm... so I guess I could check that it didn't return any error and if
> it did, then flags = 0?
>
> Something like this? (on top of this patch):
> [snip]

yeah that's obviously the easy thing to do - I just wasn't sure that
you didn't have a reason to believe it could never be an ERR_PTR :)

johannes

2017-09-06 10:45:47

by Grumbach, Emmanuel

[permalink] [raw]
Subject: [PATCH v2] cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}

Honor the NL80211_RRF_NO_HT40{MINUS,PLUS} flags in
reg_process_ht_flags_channel. Not doing so leads can lead
to a firmware assert in iwlwifi for example.

Fixes: b0d7aa59592b ("cfg80211: allow wiphy specific regdomain management")
Signed-off-by: Emmanuel Grumbach <[email protected]>
---
v2: check that regd is not ERR_PTR
---
net/wireless/reg.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 405e14366b86..dda576f5cba6 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -4,6 +4,7 @@
* Copyright 2007 Johannes Berg <[email protected]>
* Copyright 2008-2011 Luis R. Rodriguez <[email protected]>
* Copyright 2013-2014 Intel Mobile Communications GmbH
+ * Copyright 2017 Intel Deutschland GmbH
*
* Permission to use, copy, modify, and/or distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -1483,7 +1484,9 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
{
struct ieee80211_supported_band *sband = wiphy->bands[channel->band];
struct ieee80211_channel *channel_before = NULL, *channel_after = NULL;
+ const struct ieee80211_regdomain *regd;
unsigned int i;
+ u32 flags;

if (!is_ht40_allowed(channel)) {
channel->flags |= IEEE80211_CHAN_NO_HT40;
@@ -1503,17 +1506,30 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
channel_after = c;
}

+ flags = 0;
+ regd = get_wiphy_regdom(wiphy);
+ if (regd) {
+ const struct ieee80211_reg_rule *reg_rule =
+ freq_reg_info_regd(MHZ_TO_KHZ(channel->center_freq),
+ regd, MHZ_TO_KHZ(20));
+
+ if (!IS_ERR(reg_rule))
+ flags = reg_rule->flags;
+ }
+
/*
* Please note that this assumes target bandwidth is 20 MHz,
* if that ever changes we also need to change the below logic
* to include that as well.
*/
- if (!is_ht40_allowed(channel_before))
+ if (!is_ht40_allowed(channel_before) ||
+ flags & NL80211_RRF_NO_HT40MINUS)
channel->flags |= IEEE80211_CHAN_NO_HT40MINUS;
else
channel->flags &= ~IEEE80211_CHAN_NO_HT40MINUS;

- if (!is_ht40_allowed(channel_after))
+ if (!is_ht40_allowed(channel_after) ||
+ flags & NL80211_RRF_NO_HT40PLUS)
channel->flags |= IEEE80211_CHAN_NO_HT40PLUS;
else
channel->flags &= ~IEEE80211_CHAN_NO_HT40PLUS;
--
2.9.3

2017-09-06 07:52:05

by Grumbach, Emmanuel

[permalink] [raw]
Subject: RE: [PATCH 06/10] cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}

PiANCj4gT24gVHVlLCAyMDE3LTA5LTA1IGF0IDE2OjQ5ICswMDAwLCBHcnVtYmFjaCwgRW1tYW51
ZWwgd3JvdGU6DQo+ID4gT24gVHVlLCAyMDE3LTA5LTA1IGF0IDE2OjMwICswMjAwLCBKb2hhbm5l
cyBCZXJnIHdyb3RlOg0KPiA+ID4gT24gU2F0LCAyMDE3LTA4LTA1IGF0IDExOjQ0ICswMzAwLCBM
dWNhIENvZWxobyB3cm90ZToNCj4gPiA+DQo+ID4gPiA+ICsJcmVnZCA9IGdldF93aXBoeV9yZWdk
b20od2lwaHkpOw0KPiA+ID4gPiArCWlmIChyZWdkKSB7DQo+ID4gPiA+ICsJCWNvbnN0IHN0cnVj
dCBpZWVlODAyMTFfcmVnX3J1bGUgKnJlZ19ydWxlID0NCj4gPiA+ID4gKwkJCWZyZXFfcmVnX2lu
Zm9fcmVnZChNSFpfVE9fS0haKGNoYW5uZWwtDQo+ID4gPiA+ID4gY2VudGVyX2ZyZXEpLA0KPiA+
ID4gPg0KPiA+ID4gPiArCQkJCQnCoMKgwqByZWdkLA0KPiA+ID4gPiBNSFpfVE9fS0haKDIwKSk7
DQo+ID4gPiA+DQo+ID4gPg0KPiA+ID4gVGhpcyBjb3VsZCByZXR1cm4gYW4gZXJyb3IsIGhvdyBj
YW4geW91IGJlIHN1cmUgaXQgZG9lc24ndD8NCj4gPiA+DQo+ID4NCj4gPiBIbS4uLiBzbyBJIGd1
ZXNzIEkgY291bGQgY2hlY2sgdGhhdCBpdCBkaWRuJ3QgcmV0dXJuIGFueSBlcnJvciBhbmQgaWYN
Cj4gPiBpdCBkaWQsIHRoZW4gZmxhZ3MgPSAwPw0KPiA+DQo+ID4gU29tZXRoaW5nIGxpa2UgdGhp
cz8gKG9uIHRvcCBvZiB0aGlzIHBhdGNoKToNCj4gPiBbc25pcF0NCj4gDQo+IHllYWggdGhhdCdz
IG9idmlvdXNseSB0aGUgZWFzeSB0aGluZyB0byBkbyAtIEkganVzdCB3YXNuJ3Qgc3VyZSB0aGF0
IHlvdSBkaWRuJ3QNCj4gaGF2ZSBhIHJlYXNvbiB0byBiZWxpZXZlIGl0IGNvdWxkIG5ldmVyIGJl
IGFuIEVSUl9QVFIgOikNCj4NClRCSCwgSSBkb24ndCByZWFsbHkga25vdy4uLiBJIGRvbid0IGtu
b3cgaWYgKmFsbCogdGhlIGNoYW5uZWxzIGFyZSBjb3ZlcmVkIGJ5IHJ1bGVzLg0KRm9yIHN1cmUs
IGlmIHRoZXJlIGlzIG5vIHJ1bGUsIEkgY2FuIGFzc3VtZSB0aGF0IHRoZSBIVDQweyssLX0gYXJl
IG5vdCBzZXQuLi4gQnV0IEkgZG9uJ3Qga25vdyBpZiB0aGF0J3MgZXZlbiBwb3NzaWJsZS4NCg0K

2017-09-05 14:30:16

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 06/10] cfg80211: honor NL80211_RRF_NO_HT40{MINUS,PLUS}

On Sat, 2017-08-05 at 11:44 +0300, Luca Coelho wrote:

> + regd = get_wiphy_regdom(wiphy);
> + if (regd) {
> + const struct ieee80211_reg_rule *reg_rule =
> + freq_reg_info_regd(MHZ_TO_KHZ(channel-
> >center_freq),
> +    regd, MHZ_TO_KHZ(20));
>

This could return an error, how can you be sure it doesn't?

johannes

2017-09-05 14:29:09

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/10] mac80211 patches from our internal tree 2017-08-05

On Sat, 2017-08-05 at 11:44 +0300, Luca Coelho wrote:
>
> Liad Kaufman (2):
>   mac80211: add MESH IE in the correct order

applied.

> Naftali Goldstein (1):
>   mac80211: add api to start ba session timer expired flow

you already had this merged

> Sharon Dvir (1):
>   mac80211: shorten debug prints using ht_dbg() to avoid warning

applied

johannes

2017-09-06 10:29:31

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 05/10] ieee80211: Add WFA TPC report element OUI type

On Sat, 2017-08-05 at 11:44 +0300, Luca Coelho wrote:
> From: Avraham Stern <[email protected]>
>
> Signed-off-by: Avraham Stern <[email protected]>
> Signed-off-by: Luca Coelho <[email protected]>

A commit log would be good :)

johannes

2017-09-29 11:21:23

by Luca Coelho

[permalink] [raw]
Subject: [PATCH v2] ieee80211: Add WFA TPC report element OUI type

From: Avraham Stern <[email protected]>

Add Transmit Power Control OUI type definition for WLAN_OUI_MICROSOFT.

Signed-off-by: Avraham Stern <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
include/linux/ieee80211.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 55a604ad459f..ee6657a0ed69 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2445,6 +2445,7 @@ enum ieee80211_sa_query_action {
#define WLAN_OUI_TYPE_MICROSOFT_WPA 1
#define WLAN_OUI_TYPE_MICROSOFT_WMM 2
#define WLAN_OUI_TYPE_MICROSOFT_WPS 4
+#define WLAN_OUI_TYPE_MICROSOFT_TPC 8

/*
* WMM/802.11e Tspec Element
--
2.14.1

2017-09-29 11:24:41

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 09/10] mac80211_hwsim: Add command to control rx status RSSI

On Wed, 2017-09-06 at 12:33 +0200, Johannes Berg wrote:
> On Sat, 2017-08-05 at 11:44 +0300, Luca Coelho wrote:
> >
> > +static int hwsim_fops_rx_rssi_write(void *dat, u64 val)
> > +{
> > + struct mac80211_hwsim_data *data = dat;
> > + int rssi = (int)val;
>
> That cast is wrong - you should go to s64, do the range check, and
> only
> then restrict to int. Basically, just do
>
> s64 rssi = val;
>
> in this line instead.
>
>
> > + debugfs_create_file("rx_rssi", 0666, data->debugfs, data,
> > + &hwsim_fops_rx_rssi);
>
> I also can't say I'm really happy with this, since there's no such
> thing as an "RX RSSI".
>
> Also, wmediumd already allows changing this - I'd rather people start
> using the infrastructure better than papering over more.
>
> IOW - do we really need this? Why can't you do the (simple) wmediumd
> setup? There's even a hwsim test in wpa_s with wmediumd, it's not all
> that difficult.

Beni,

Can you answer Johannes here? I'm removing this patch from my pending
list, so I won't take any action anymore until further notice.

--
Cheers,
Luca.