* Re: [PATCH] cfg80211: Add new helper function for channels
From: Arend Van Spriel @ 2019-08-30 10:40 UTC (permalink / raw)
To: Amar Singhal, jouni; +Cc: johannes, linux-wireless, jjohnson, rmanohar
In-Reply-To: <1567115381-7831-1-git-send-email-asinghal@codeaurora.org>
On 8/29/2019 11:49 PM, Amar Singhal wrote:
> Add new helper function to convert (chan_number, oper_class) pair to
> frequency. Call this function ieee80211_channel_op_class_to_frequency.
> This function would be very useful in the context of 6 GHz channels,
> where channel number is not unique.
That 'unique' statement does not apply to 6GHz by itself. The addition
of 6GHz channels makes channel numbers across bands not unique.
The funcion
> Signed-off-by: Amar Singhal <asinghal@codeaurora.org>
> ---
> include/net/cfg80211.h | 10 ++++++++++
> net/wireless/util.c | 23 +++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 6467b60..decafba 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -4914,1 +4914,1 @@ static inline void *wdev_priv(struct wireless_dev *wdev)
> int ieee80211_channel_to_frequency(int chan, enum nl80211_band band);
>
> /**
> + * ieee80211_channel_op_class_to_frequency - convert
> + * (channel, operating class) to frequency
> + * @chan_num: channel number
> + * @global_op_class: global operating class
> + *
> + * Return: The corresponding frequency, or 0 if the conversion failed.
> + */
> +int ieee80211_channel_op_class_to_frequency(u8 chan_num, u8 global_op_class);
> +
> +/**
> * ieee80211_frequency_to_channel - convert frequency to channel number
> * @freq: center frequency
> * Return: The corresponding channel, or 0 if the conversion failed.
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 9aba8d54..7f64b4a 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -144,6 +144,29 @@ struct ieee80211_channel *ieee80211_get_channel(struct wiphy *wiphy, int freq)
> }
> EXPORT_SYMBOL(ieee80211_get_channel);
>
> +int ieee80211_channel_op_class_to_frequency(u8 chan_num, u8 global_op_class)
> +{
> + if (global_op_class >= 131 && global_op_class <= 135)
> + return (5940 + 5 * chan_num);
> + else if (global_op_class >= 115 && global_op_class <= 130)
> + return (5000 + 5 * chan_num);
> + else if (global_op_class >= 112 && global_op_class <= 113)
> + return (5000 + 5 * chan_num);
> + else if (global_op_class >= 109 && global_op_class <= 110)
> + return (4000 + 5 * chan_num);
> + else if (global_op_class >= 83 && global_op_class <= 84)
> + return (2407 + 5 * chan_num);
> + else if (global_op_class == 81)
> + return (2407 + 5 * chan_num);
> + else if (global_op_class == 82)
> + return (2414 + 5 * chan_num);
> + else if (global_op_class == 180)
> + return (56160 + 5 * chan_num);
> + else
> + return 0;
> +}
> +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
The function ieee80211_operating_class_to_band() uses ranges within
switch statement, eg.:
case 128 ... 130:
*band = NL80211_BAND_5GHZ;
return true;
For consistency it might be good to do the same here.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH] cfg80211: Add new helper function for channels
From: Johannes Berg @ 2019-08-30 10:41 UTC (permalink / raw)
To: Arend Van Spriel, Amar Singhal, jouni; +Cc: linux-wireless, jjohnson, rmanohar
In-Reply-To: <01082dca-8d71-c674-1f61-ab5d7e07c007@broadcom.com>
On Fri, 2019-08-30 at 12:40 +0200, Arend Van Spriel wrote:
> > +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
>
> The function ieee80211_operating_class_to_band() uses ranges within
> switch statement, eg.:
>
> case 128 ... 130:
> *band = NL80211_BAND_5GHZ;
> return true;
No that you remind me - how is this new function not just a composition
of the existing ones?
i.e. just convert the op_class to band first, and then (band, channel)
to freq?
johannes
^ permalink raw reply
* Re: [PATCH] cfg80211: Convert 6 GHz channel frequency to channel number
From: Arend Van Spriel @ 2019-08-30 10:45 UTC (permalink / raw)
To: Amar Singhal, johannes, jouni; +Cc: linux-wireless, jjohnson, rmanohar
In-Reply-To: <1567117290-19295-1-git-send-email-asinghal@codeaurora.org>
On 8/30/2019 12:21 AM, Amar Singhal wrote:
> Enhance function ieee80211_frequency_to_channel by adding 6 GHz
> channels.
This look very similar to what I submitted earlier:
https://patchwork.kernel.org/patch/11073197/
Regards,
Arend
^ permalink raw reply
* Re: [PATCH] cfg80211: Convert 6 GHz channel frequency to channel number
From: Arend Van Spriel @ 2019-08-30 10:47 UTC (permalink / raw)
To: Johannes Berg, Amar Singhal, jouni; +Cc: linux-wireless, jjohnson, rmanohar
In-Reply-To: <420dcc9c0023791b72dba2d776045fc0b025597a.camel@sipsolutions.net>
On 8/30/2019 12:32 PM, Johannes Berg wrote:
> On Thu, 2019-08-29 at 15:21 -0700, Amar Singhal wrote:
>> Enhance function ieee80211_frequency_to_channel by adding 6 GHz
>> channels.
>
> Wait, this is already supported, no? Just implemented slightly
> differently?
It is Johannes, but I was unaware as well. Did you forget to email that
it was applied or is there some automated stuff that failed on you? ;-)
Regards,
Arend
^ permalink raw reply
* Re: [PATCH] cfg80211: Convert 6 GHz channel frequency to channel number
From: Johannes Berg @ 2019-08-30 10:48 UTC (permalink / raw)
To: Arend Van Spriel, Amar Singhal, jouni; +Cc: linux-wireless, jjohnson, rmanohar
In-Reply-To: <b9520a71-2571-4334-65b8-8707159406aa@broadcom.com>
On Fri, 2019-08-30 at 12:47 +0200, Arend Van Spriel wrote:
> On 8/30/2019 12:32 PM, Johannes Berg wrote:
> > On Thu, 2019-08-29 at 15:21 -0700, Amar Singhal wrote:
> > > Enhance function ieee80211_frequency_to_channel by adding 6 GHz
> > > channels.
> >
> > Wait, this is already supported, no? Just implemented slightly
> > differently?
>
> It is Johannes, but I was unaware as well. Did you forget to email that
> it was applied or is there some automated stuff that failed on you? ;-)
Truth be told, I've been very lazy (mostly due to being busy) and
haven't responded manually - and also haven't managed to set up anything
that would automate a response, I tried Kalle's tool at one point but it
didn't work for me yet.
Right now, your best bet is probably to poll patchwork, sorry about
that.
johannes
^ permalink raw reply
* Re: [PATCH] cfg80211: Add new helper function for channels
From: Arend Van Spriel @ 2019-08-30 11:03 UTC (permalink / raw)
To: Johannes Berg, Amar Singhal, jouni; +Cc: linux-wireless, jjohnson, rmanohar
In-Reply-To: <6222b9d9d125c4c3a9f60cc0fe73cef2011b9959.camel@sipsolutions.net>
On 8/30/2019 12:41 PM, Johannes Berg wrote:
> On Fri, 2019-08-30 at 12:40 +0200, Arend Van Spriel wrote:
>
>>> +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
>>
>> The function ieee80211_operating_class_to_band() uses ranges within
>> switch statement, eg.:
>>
>> case 128 ... 130:
>> *band = NL80211_BAND_5GHZ;
>> return true;
>
> No that you remind me - how is this new function not just a composition
> of the existing ones?
>
> i.e. just convert the op_class to band first, and then (band, channel)
> to freq?
yup. that would have my preference actually.
Regards,
Arend
^ permalink raw reply
* [PATCH 0/8] cfg80211/mac80211 patches from our internal tree 2019-08-30
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
From: Luca Coelho <luciano.coelho@intel.com>
Hi,
Some patches with mac80211 and cfg80211 changes from our internal
tree.
Please review, though you have already reviewed most if not all of
them ;)
Cheers,
Luca.
Ilan Peer (1):
mac80211: Allow user space to register for station Rx authentication
Johannes Berg (4):
cfg80211: always shut down on HW rfkill
mac80211: list features in WEP/TKIP disable in better order
mac80211: remove unnecessary key condition
mac80211: IBSS: send deauth when expiring inactive STAs
Lior Cohen (1):
mac80211: clear crypto tx tailroom counter upon keys enable
Luca Coelho (1):
mac80211: don't check if key is NULL in ieee80211_key_link()
Mordechay Goodstein (1):
mac80211: vht: add support VHT EXT NSS BW in parsing VHT
net/mac80211/ibss.c | 8 +++++++
net/mac80211/ieee80211_i.h | 3 ++-
net/mac80211/key.c | 48 +++++++++++++-------------------------
net/mac80211/key.h | 4 ++--
net/mac80211/main.c | 13 +++++++++++
net/mac80211/mlme.c | 13 ++++++-----
net/mac80211/util.c | 11 ++++-----
net/mac80211/vht.c | 10 +++++++-
net/wireless/core.c | 13 ++++++-----
net/wireless/core.h | 2 +-
net/wireless/wext-compat.c | 5 ++--
11 files changed, 72 insertions(+), 58 deletions(-)
--
2.23.0.rc1
^ permalink raw reply
* [PATCH 1/8] cfg80211: always shut down on HW rfkill
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
From: Johannes Berg <johannes.berg@intel.com>
When the RFKILL subsystem isn't available, then rfkill_blocked()
always returns false. In the case of hardware rfkill this will
be wrong though, as if the hardware reported being killed then
it cannot operate any longer.
Since we only ever call the rfkill_sync work in this case, just
rename it to rfkill_block and always pass "true" for the blocked
parameter, rather than passing rfkill_blocked().
We rely on the underlying driver to still reject any new attempt
to bring up the device by itself.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/wireless/core.c | 13 +++++++------
net/wireless/core.h | 2 +-
net/wireless/wext-compat.c | 5 +++--
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index a599469b8157..350513744575 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -5,7 +5,7 @@
* Copyright 2006-2010 Johannes Berg <johannes@sipsolutions.net>
* Copyright 2013-2014 Intel Mobile Communications GmbH
* Copyright 2015-2017 Intel Deutschland GmbH
- * Copyright (C) 2018 Intel Corporation
+ * Copyright (C) 2018-2019 Intel Corporation
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -300,12 +300,13 @@ static int cfg80211_rfkill_set_block(void *data, bool blocked)
return 0;
}
-static void cfg80211_rfkill_sync_work(struct work_struct *work)
+static void cfg80211_rfkill_block_work(struct work_struct *work)
{
struct cfg80211_registered_device *rdev;
- rdev = container_of(work, struct cfg80211_registered_device, rfkill_sync);
- cfg80211_rfkill_set_block(rdev, rfkill_blocked(rdev->rfkill));
+ rdev = container_of(work, struct cfg80211_registered_device,
+ rfkill_block);
+ cfg80211_rfkill_set_block(rdev, true);
}
static void cfg80211_event_work(struct work_struct *work)
@@ -516,7 +517,7 @@ struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv,
return NULL;
}
- INIT_WORK(&rdev->rfkill_sync, cfg80211_rfkill_sync_work);
+ INIT_WORK(&rdev->rfkill_block, cfg80211_rfkill_block_work);
INIT_WORK(&rdev->conn_work, cfg80211_conn_work);
INIT_WORK(&rdev->event_work, cfg80211_event_work);
@@ -1061,7 +1062,7 @@ void wiphy_rfkill_set_hw_state(struct wiphy *wiphy, bool blocked)
struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
if (rfkill_set_hw_state(rdev->rfkill, blocked))
- schedule_work(&rdev->rfkill_sync);
+ schedule_work(&rdev->rfkill_block);
}
EXPORT_SYMBOL(wiphy_rfkill_set_hw_state);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 77556c58d9ac..ed487e324571 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -28,7 +28,7 @@ struct cfg80211_registered_device {
/* rfkill support */
struct rfkill_ops rfkill_ops;
struct rfkill *rfkill;
- struct work_struct rfkill_sync;
+ struct work_struct rfkill_block;
/* ISO / IEC 3166 alpha2 for which this device is receiving
* country IEs on, this can help disregard country IEs from APs
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 46e4d69db845..7b6529d81c61 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -7,6 +7,7 @@
* we directly assign the wireless handlers of wireless interfaces.
*
* Copyright 2008-2009 Johannes Berg <johannes@sipsolutions.net>
+ * Copyright (C) 2019 Intel Corporation
*/
#include <linux/export.h>
@@ -864,8 +865,8 @@ static int cfg80211_wext_siwtxpower(struct net_device *dev,
}
}
} else {
- rfkill_set_sw_state(rdev->rfkill, true);
- schedule_work(&rdev->rfkill_sync);
+ if (rfkill_set_sw_state(rdev->rfkill, true))
+ schedule_work(&rdev->rfkill_block);
return 0;
}
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH 4/8] mac80211: Allow user space to register for station Rx authentication
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
From: Ilan Peer <ilan.peer@intel.com>
To support Pre Association Security Negotiation (PASN) while already
associated to one AP, allow user space to register to Rx authentication
frames, so that the user space logic would be able to receive/handle
authentication frames from a different AP as part of PASN.
Note that it is expected that user space would intelligently register
for Rx authentication frames, i.e., only when PASN is used and configure
a match filter only for PASN authentication algorithm, as otherwise
the MLME functionality of mac80211 would be broken.
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/mac80211/main.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 29b9d57df1a3..6eddc5592fc4 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -416,7 +416,20 @@ ieee80211_default_mgmt_stypes[NUM_NL80211_IFTYPES] = {
},
[NL80211_IFTYPE_STATION] = {
.tx = 0xffff,
+ /*
+ * To support Pre Association Security Negotiation (PASN) while
+ * already associated to one AP, allow user space to register to
+ * Rx authentication frames, so that the user space logic would
+ * be able to receive/handle authentication frames from a
+ * different AP as part of PASN.
+ * It is expected that user space would intelligently register
+ * for Rx authentication frames, i.e., only when PASN is used
+ * and configure a match filter only for PASN authentication
+ * algorithm, as otherwise the MLME functionality of mac80211
+ * would be broken.
+ */
.rx = BIT(IEEE80211_STYPE_ACTION >> 4) |
+ BIT(IEEE80211_STYPE_AUTH >> 4) |
BIT(IEEE80211_STYPE_PROBE_REQ >> 4),
},
[NL80211_IFTYPE_AP] = {
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH 6/8] mac80211: clear crypto tx tailroom counter upon keys enable
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
From: Lior Cohen <lior2.cohen@intel.com>
In case we got a fw restart while roaming from encrypted AP to
non-encrypted one, we might end up with hitting a warning on the pending
counter crypto_tx_tailroom_pending_dec having a non-zero value.
The following comment taken from net/mac80211/key.c explains the rational
for the delayed tailroom needed:
/*
* The reason for the delayed tailroom needed decrementing is to
* make roaming faster: during roaming, all keys are first deleted
* and then new keys are installed. The first new key causes the
* crypto_tx_tailroom_needed_cnt to go from 0 to 1, which invokes
* the cost of synchronize_net() (which can be slow). Avoid this
* by deferring the crypto_tx_tailroom_needed_cnt decrementing on
* key removal for a while, so if we roam the value is larger than
* zero and no 0->1 transition happens.
*
* The cost is that if the AP switching was from an AP with keys
* to one without, we still allocate tailroom while it would no
* longer be needed. However, in the typical (fast) roaming case
* within an ESS this usually won't happen.
*/
The next flow lead to the warning eventually reported as a bug:
1. Disconnect from encrypted AP
2. Set crypto_tx_tailroom_pending_dec = 1 for the key
3. Schedule work
4. Reconnect to non-encrypted AP
5. Add a new key, setting the tailroom counter = 1
6. Got FW restart while pending counter is set ---> hit the warning
While on it, the ieee80211_reset_crypto_tx_tailroom() func was merged into
its single caller ieee80211_reenable_keys (previously called
ieee80211_enable_keys). Also, we reset the crypto_tx_tailroom_pending_dec
and remove the counters warning as we just reset both.
Signed-off-by: Lior Cohen <lior2.cohen@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/mac80211/key.c | 40 ++++++++++++----------------------------
net/mac80211/key.h | 4 ++--
net/mac80211/util.c | 6 +-----
3 files changed, 15 insertions(+), 35 deletions(-)
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 1be3686562ee..93ea03b86b80 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -843,46 +843,30 @@ void ieee80211_key_free(struct ieee80211_key *key, bool delay_tailroom)
ieee80211_key_destroy(key, delay_tailroom);
}
-void ieee80211_enable_keys(struct ieee80211_sub_if_data *sdata)
+void ieee80211_reenable_keys(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_key *key;
struct ieee80211_sub_if_data *vlan;
ASSERT_RTNL();
- if (WARN_ON(!ieee80211_sdata_running(sdata)))
- return;
-
- mutex_lock(&sdata->local->key_mtx);
-
- WARN_ON_ONCE(sdata->crypto_tx_tailroom_needed_cnt ||
- sdata->crypto_tx_tailroom_pending_dec);
-
- if (sdata->vif.type == NL80211_IFTYPE_AP) {
- list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list)
- WARN_ON_ONCE(vlan->crypto_tx_tailroom_needed_cnt ||
- vlan->crypto_tx_tailroom_pending_dec);
- }
-
- list_for_each_entry(key, &sdata->key_list, list) {
- increment_tailroom_need_count(sdata);
- ieee80211_key_enable_hw_accel(key);
- }
-
- mutex_unlock(&sdata->local->key_mtx);
-}
-
-void ieee80211_reset_crypto_tx_tailroom(struct ieee80211_sub_if_data *sdata)
-{
- struct ieee80211_sub_if_data *vlan;
-
mutex_lock(&sdata->local->key_mtx);
sdata->crypto_tx_tailroom_needed_cnt = 0;
+ sdata->crypto_tx_tailroom_pending_dec = 0;
if (sdata->vif.type == NL80211_IFTYPE_AP) {
- list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list)
+ list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list) {
vlan->crypto_tx_tailroom_needed_cnt = 0;
+ vlan->crypto_tx_tailroom_pending_dec = 0;
+ }
+ }
+
+ if (ieee80211_sdata_running(sdata)) {
+ list_for_each_entry(key, &sdata->key_list, list) {
+ increment_tailroom_need_count(sdata);
+ ieee80211_key_enable_hw_accel(key);
+ }
}
mutex_unlock(&sdata->local->key_mtx);
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index b8b9cd743bf4..d6d6e89cf7dd 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -2,6 +2,7 @@
/*
* Copyright 2002-2004, Instant802 Networks, Inc.
* Copyright 2005, Devicescape Software, Inc.
+ * Copyright (C) 2019 Intel Corporation
*/
#ifndef IEEE80211_KEY_H
@@ -156,8 +157,7 @@ void ieee80211_free_keys(struct ieee80211_sub_if_data *sdata,
bool force_synchronize);
void ieee80211_free_sta_keys(struct ieee80211_local *local,
struct sta_info *sta);
-void ieee80211_enable_keys(struct ieee80211_sub_if_data *sdata);
-void ieee80211_reset_crypto_tx_tailroom(struct ieee80211_sub_if_data *sdata);
+void ieee80211_reenable_keys(struct ieee80211_sub_if_data *sdata);
#define key_mtx_dereference(local, ref) \
rcu_dereference_protected(ref, lockdep_is_held(&((local)->key_mtx)))
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 286c7ee35e63..92bfedfd3fd2 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2420,11 +2420,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
/* add back keys */
list_for_each_entry(sdata, &local->interfaces, list)
- ieee80211_reset_crypto_tx_tailroom(sdata);
-
- list_for_each_entry(sdata, &local->interfaces, list)
- if (ieee80211_sdata_running(sdata))
- ieee80211_enable_keys(sdata);
+ ieee80211_reenable_keys(sdata);
/* Reconfigure sched scan if it was interrupted by FW restart */
mutex_lock(&local->mtx);
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH 5/8] mac80211: remove unnecessary key condition
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
From: Johannes Berg <johannes.berg@intel.com>
When we reach this point, the key cannot be NULL. Remove the condition
that suggests otherwise.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/mac80211/key.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 7dfee848abac..1be3686562ee 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -6,6 +6,7 @@
* Copyright 2007-2008 Johannes Berg <johannes@sipsolutions.net>
* Copyright 2013-2014 Intel Mobile Communications GmbH
* Copyright 2015-2017 Intel Deutschland GmbH
+ * Copyright 2018-2019 Intel Corporation
*/
#include <linux/if_ether.h>
@@ -781,9 +782,8 @@ int ieee80211_key_link(struct ieee80211_key *key,
/* The rekey code assumes that the old and new key are using
* the same cipher. Enforce the assumption for pairwise keys.
*/
- if (key &&
- ((alt_key && alt_key->conf.cipher != key->conf.cipher) ||
- (old_key && old_key->conf.cipher != key->conf.cipher)))
+ if ((alt_key && alt_key->conf.cipher != key->conf.cipher) ||
+ (old_key && old_key->conf.cipher != key->conf.cipher))
goto out;
} else if (sta) {
old_key = key_mtx_dereference(sdata->local, sta->gtk[idx]);
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH 7/8] mac80211: don't check if key is NULL in ieee80211_key_link()
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
From: Luca Coelho <luciano.coelho@intel.com>
We already assume that key is not NULL and dereference it in a few
other places before we check whether it is NULL, so the check is
unnecessary. Remove it.
Fixes: 96fc6efb9ad9 ("mac80211: IEEE 802.11 Extended Key ID support")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/mac80211/key.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 93ea03b86b80..0f889b919b06 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -793,7 +793,7 @@ int ieee80211_key_link(struct ieee80211_key *key,
/* Non-pairwise keys must also not switch the cipher on rekey */
if (!pairwise) {
- if (key && old_key && old_key->conf.cipher != key->conf.cipher)
+ if (old_key && old_key->conf.cipher != key->conf.cipher)
goto out;
}
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH 3/8] mac80211: list features in WEP/TKIP disable in better order
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
From: Johannes Berg <johannes.berg@intel.com>
"HE/HT/VHT" is a bit confusing since really the order of
development (and possible support) is different - change
this to "HT/VHT/HE".
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/mac80211/mlme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 6471f552a942..31f0bae28dcc 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5296,7 +5296,7 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
netdev_info(sdata->dev,
- "disabling HE/HT/VHT due to WEP/TKIP use\n");
+ "disabling HT/VHT/HE due to WEP/TKIP use\n");
}
}
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH 8/8] mac80211: IBSS: send deauth when expiring inactive STAs
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
From: Johannes Berg <johannes.berg@intel.com>
When we expire an inactive station, try to send it a deauth. This
helps if it's actually still around, and just has issues with
beacon distribution (or we do), and it will not also remove us.
Then, if we have shared state, this may not be reset properly,
causing problems; for example, we saw a case where aggregation
sessions weren't removed properly (due to the TX start being
offloaded to firmware and it relying on deauth for stop), causing
a lot of traffic to get lost due to the SN reset after remove/add
of the peer.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/mac80211/ibss.c | 8 ++++++++
net/mac80211/ieee80211_i.h | 3 ++-
net/mac80211/mlme.c | 11 ++++++-----
net/mac80211/util.c | 5 +++--
4 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index f00dca056295..0a6ff01c68a9 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -1252,6 +1252,7 @@ void ieee80211_ibss_rx_no_sta(struct ieee80211_sub_if_data *sdata,
static void ieee80211_ibss_sta_expire(struct ieee80211_sub_if_data *sdata)
{
+ struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
struct ieee80211_local *local = sdata->local;
struct sta_info *sta, *tmp;
unsigned long exp_time = IEEE80211_IBSS_INACTIVITY_LIMIT;
@@ -1268,10 +1269,17 @@ static void ieee80211_ibss_sta_expire(struct ieee80211_sub_if_data *sdata)
if (time_is_before_jiffies(last_active + exp_time) ||
(time_is_before_jiffies(last_active + exp_rsn) &&
sta->sta_state != IEEE80211_STA_AUTHORIZED)) {
+ u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
+
sta_dbg(sta->sdata, "expiring inactive %sSTA %pM\n",
sta->sta_state != IEEE80211_STA_AUTHORIZED ?
"not authorized " : "", sta->sta.addr);
+ ieee80211_send_deauth_disassoc(sdata, sta->sta.addr,
+ ifibss->bssid,
+ IEEE80211_STYPE_DEAUTH,
+ WLAN_REASON_DEAUTH_LEAVING,
+ true, frame_buf);
WARN_ON(__sta_info_destroy(sta));
}
}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 791ce58d0f09..05406e9c05b3 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2099,7 +2099,8 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
const u8 *da, const u8 *key, u8 key_len, u8 key_idx,
u32 tx_flags);
void ieee80211_send_deauth_disassoc(struct ieee80211_sub_if_data *sdata,
- const u8 *bssid, u16 stype, u16 reason,
+ const u8 *da, const u8 *bssid,
+ u16 stype, u16 reason,
bool send_frame, u8 *frame_buf);
enum {
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 31f0bae28dcc..26a2f49208b6 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2278,8 +2278,9 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
!ifmgd->have_beacon)
drv_mgd_prepare_tx(sdata->local, sdata, 0);
- ieee80211_send_deauth_disassoc(sdata, ifmgd->bssid, stype,
- reason, tx, frame_buf);
+ ieee80211_send_deauth_disassoc(sdata, ifmgd->bssid,
+ ifmgd->bssid, stype, reason,
+ tx, frame_buf);
}
/* flush out frame - make sure the deauth was actually sent */
@@ -4509,7 +4510,7 @@ void ieee80211_mgd_quiesce(struct ieee80211_sub_if_data *sdata)
* cfg80211 won't know and won't actually abort those attempts,
* thus we need to do that ourselves.
*/
- ieee80211_send_deauth_disassoc(sdata, bssid,
+ ieee80211_send_deauth_disassoc(sdata, bssid, bssid,
IEEE80211_STYPE_DEAUTH,
WLAN_REASON_DEAUTH_LEAVING,
false, frame_buf);
@@ -5550,7 +5551,7 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
ieee80211_get_reason_code_string(req->reason_code));
drv_mgd_prepare_tx(sdata->local, sdata, 0);
- ieee80211_send_deauth_disassoc(sdata, req->bssid,
+ ieee80211_send_deauth_disassoc(sdata, req->bssid, req->bssid,
IEEE80211_STYPE_DEAUTH,
req->reason_code, tx,
frame_buf);
@@ -5570,7 +5571,7 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
ieee80211_get_reason_code_string(req->reason_code));
drv_mgd_prepare_tx(sdata->local, sdata, 0);
- ieee80211_send_deauth_disassoc(sdata, req->bssid,
+ ieee80211_send_deauth_disassoc(sdata, req->bssid, req->bssid,
IEEE80211_STYPE_DEAUTH,
req->reason_code, tx,
frame_buf);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 92bfedfd3fd2..051a02ddcb85 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1583,7 +1583,8 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
}
void ieee80211_send_deauth_disassoc(struct ieee80211_sub_if_data *sdata,
- const u8 *bssid, u16 stype, u16 reason,
+ const u8 *da, const u8 *bssid,
+ u16 stype, u16 reason,
bool send_frame, u8 *frame_buf)
{
struct ieee80211_local *local = sdata->local;
@@ -1594,7 +1595,7 @@ void ieee80211_send_deauth_disassoc(struct ieee80211_sub_if_data *sdata,
mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT | stype);
mgmt->duration = 0; /* initialize only */
mgmt->seq_ctrl = 0; /* initialize only */
- memcpy(mgmt->da, bssid, ETH_ALEN);
+ memcpy(mgmt->da, da, ETH_ALEN);
memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
memcpy(mgmt->bssid, bssid, ETH_ALEN);
/* u.deauth.reason_code == u.disassoc.reason_code */
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH 1/8] cfg80211: always shut down on HW rfkill
From: Luca Coelho @ 2019-08-30 11:24 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-1-luca@coelho.fi>
This fixes was missed in parsing the vht capabilities max bw
support.
Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.com>
Fixes: e80d642552a3 ("mac80211: copy VHT EXT NSS BW Support/Capable data to station")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
net/mac80211/vht.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index b20ff28d9f30..ccdcb9ad9ac7 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -4,7 +4,7 @@
*
* Portions of this file
* Copyright(c) 2015 - 2016 Intel Deutschland GmbH
- * Copyright (C) 2018 Intel Corporation
+ * Copyright (C) 2018 - 2019 Intel Corporation
*/
#include <linux/ieee80211.h>
@@ -349,6 +349,14 @@ enum ieee80211_sta_rx_bandwidth ieee80211_sta_cap_rx_bw(struct sta_info *sta)
cap_width == IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)
return IEEE80211_STA_RX_BW_160;
+ /*
+ * If this is non-zero, then it does support 160 MHz after all,
+ * in one form or the other. We don't distinguish here (or even
+ * above) between 160 and 80+80 yet.
+ */
+ if (vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)
+ return IEEE80211_STA_RX_BW_160;
+
return IEEE80211_STA_RX_BW_80;
}
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2] mac80211: vht: add support VHT EXT NSS BW in parsing VHT
From: Luca Coelho @ 2019-08-30 11:40 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless
In-Reply-To: <20190830112451.21655-3-luca@coelho.fi>
From: Mordechay Goodstein <mordechay.goodstein@intel.com>
This fixes was missed in parsing the vht capabilities max bw
support.
Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.com>
Fixes: e80d642552a3 ("mac80211: copy VHT EXT NSS BW Support/Capable data to station")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
Fixed the subject in v2 (no idea what happened there).
net/mac80211/vht.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index b20ff28d9f30..ccdcb9ad9ac7 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -4,7 +4,7 @@
*
* Portions of this file
* Copyright(c) 2015 - 2016 Intel Deutschland GmbH
- * Copyright (C) 2018 Intel Corporation
+ * Copyright (C) 2018 - 2019 Intel Corporation
*/
#include <linux/ieee80211.h>
@@ -349,6 +349,14 @@ enum ieee80211_sta_rx_bandwidth ieee80211_sta_cap_rx_bw(struct sta_info *sta)
cap_width == IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)
return IEEE80211_STA_RX_BW_160;
+ /*
+ * If this is non-zero, then it does support 160 MHz after all,
+ * in one form or the other. We don't distinguish here (or even
+ * above) between 160 and 80+80 yet.
+ */
+ if (vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK)
+ return IEEE80211_STA_RX_BW_160;
+
return IEEE80211_STA_RX_BW_80;
}
--
2.23.0.rc1
^ permalink raw reply related
* Re: [PATCH] mt76: mt7615: move mt7615 sta utilities in pci.c
From: Ryder Lee @ 2019-08-30 11:52 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: nbd, lorenzo.bianconi, linux-wireless, royluo
In-Reply-To: <e095a331854ab9c9619f86ae8860502abf808fbe.1567068930.git.lorenzo@kernel.org>
On Thu, 2019-08-29 at 10:57 +0200, Lorenzo Bianconi wrote:
> Move mt7615_sta_add, mt7615_sta_assoc, mt7615_sta_ps and mt7615_sta_remove
> utility routines in pci.c and make them static since they are only used to
> initialize mt76_driver_ops callbacks
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> .../net/wireless/mediatek/mt76/mt7615/mac.c | 4 --
> .../net/wireless/mediatek/mt76/mt7615/main.c | 40 ---------------
> .../wireless/mediatek/mt76/mt7615/mt7615.h | 7 ---
> .../net/wireless/mediatek/mt76/mt7615/pci.c | 49 +++++++++++++++++++
> 4 files changed, 49 insertions(+), 51 deletions(-)
Just to note that we might have other interface shares with the same
utility routines in upcoming days.
Ryder
^ permalink raw reply
* Re: [PATCH] mt76: mt7615: move mt7615 sta utilities in pci.c
From: Lorenzo Bianconi @ 2019-08-30 12:06 UTC (permalink / raw)
To: Ryder Lee; +Cc: nbd, lorenzo.bianconi, linux-wireless, royluo
In-Reply-To: <1567165961.8079.3.camel@mtkswgap22>
[-- Attachment #1: Type: text/plain, Size: 941 bytes --]
> On Thu, 2019-08-29 at 10:57 +0200, Lorenzo Bianconi wrote:
> > Move mt7615_sta_add, mt7615_sta_assoc, mt7615_sta_ps and mt7615_sta_remove
> > utility routines in pci.c and make them static since they are only used to
> > initialize mt76_driver_ops callbacks
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > .../net/wireless/mediatek/mt76/mt7615/mac.c | 4 --
> > .../net/wireless/mediatek/mt76/mt7615/main.c | 40 ---------------
> > .../wireless/mediatek/mt76/mt7615/mt7615.h | 7 ---
> > .../net/wireless/mediatek/mt76/mt7615/pci.c | 49 +++++++++++++++++++
> > 4 files changed, 49 insertions(+), 51 deletions(-)
>
> Just to note that we might have other interface shares with the same
> utility routines in upcoming days.
>
> Ryder
>
Hi Ryder,
I guess what really matter is if they are non-PCI based devices and if they
rely on the same mcu commands
Regards,
Lorenzo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v6] mac80211_hwsim: Register support for HE meshpoint
From: Jouni Malinen @ 2019-08-30 13:37 UTC (permalink / raw)
To: Johannes Berg; +Cc: Sven Eckelmann, linux-wireless
In-Reply-To: <766f6d196106fa7ca6036b2ce20243e042d9168d.camel@sipsolutions.net>
On Fri, Aug 30, 2019 at 12:38:20PM +0200, Johannes Berg wrote:
> > > mesh_secure_ocv_mix_legacy
> > > wpas_mesh_open_ht40
> > > mesh_secure_ocv_mix_ht
>
> > No, these also failed for me without the mac80211_hwsim patch [3] in a full
> > test run. And thus not analyzed further by me.
>
> I also see these fail if (and only if) I have this patch applied.
>
> I'm going to drop this patch (again) for now, even if the situation is
> now better I still don't want to knowingly break things there.
>
> Please resend once that's all sorted out.
I pushed the relevant wpa_supplicant changes into hostap.git and all the
hwsim test cases pass now with the current snapshot when this
mac80211_hwsim patch is included.
--
Jouni Malinen PGP id EFC895FA
^ permalink raw reply
* Re: [PATCH v6] mac80211_hwsim: Register support for HE meshpoint
From: Johannes Berg @ 2019-08-30 13:45 UTC (permalink / raw)
To: Jouni Malinen; +Cc: Sven Eckelmann, linux-wireless
In-Reply-To: <20190830133737.GC21204@w1.fi>
On Fri, 2019-08-30 at 16:37 +0300, Jouni Malinen wrote:
> On Fri, Aug 30, 2019 at 12:38:20PM +0200, Johannes Berg wrote:
> > > > mesh_secure_ocv_mix_legacy
> > > > wpas_mesh_open_ht40
> > > > mesh_secure_ocv_mix_ht
> > > No, these also failed for me without the mac80211_hwsim patch [3] in a full
> > > test run. And thus not analyzed further by me.
> >
> > I also see these fail if (and only if) I have this patch applied.
> >
> > I'm going to drop this patch (again) for now, even if the situation is
> > now better I still don't want to knowingly break things there.
> >
> > Please resend once that's all sorted out.
>
> I pushed the relevant wpa_supplicant changes into hostap.git and all the
> hwsim test cases pass now with the current snapshot when this
> mac80211_hwsim patch is included.
Great, thanks. I've resurrected the patch in patchwork, so no need to
resend.
johannes
^ permalink raw reply
* Re: [PATCH] cfg80211: Add new helper function for channels
From: Jeff Johnson @ 2019-08-30 14:09 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Johannes Berg, Amar Singhal, jouni, linux-wireless, rmanohar
In-Reply-To: <9a9c8024-a9f9-72f9-557a-c185dca9c15c@broadcom.com>
On 2019-08-30 04:03, Arend Van Spriel wrote:
> On 8/30/2019 12:41 PM, Johannes Berg wrote:
>> On Fri, 2019-08-30 at 12:40 +0200, Arend Van Spriel wrote:
>>
>>>> +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
>>>
>>> The function ieee80211_operating_class_to_band() uses ranges within
>>> switch statement, eg.:
>>>
>>> case 128 ... 130:
>>> *band = NL80211_BAND_5GHZ;
>>> return true;
>>
>> No that you remind me - how is this new function not just a
>> composition
>> of the existing ones?
>>
>> i.e. just convert the op_class to band first, and then (band, channel)
>> to freq?
>
> yup. that would have my preference actually.
Sigh. I had the same guidance in pre-review:
we already have ieee80211_operating_class_to_band() and
ieee80211_channel_to_frequency() so all this function should be is
return ieee80211_channel_to_frequency(chan,
ieee80211_operating_class_to_band(op_class))
but then again if anybody needs this functionality they can simply call
those same functions
^ permalink raw reply
* Re: [PATCH] cfg80211: Purge frame registrations on iftype change
From: Denis Kenzior @ 2019-08-30 6:32 UTC (permalink / raw)
To: Johannes Berg, linux-wireless; +Cc: stable
In-Reply-To: <5dc694c33759a32eb3796668a8b396c0133e1ebe.camel@sipsolutions.net>
Hi Johannes,
On 8/30/19 3:53 AM, Johannes Berg wrote:
> On Wed, 2019-08-28 at 16:11 -0500, Denis Kenzior wrote:
>> Currently frame registrations are not purged, even when changing the
>> interface type. This can lead to potentially weird / dangerous
>> situations where frames possibly not relevant to a given interface
>> type remain registered and mgmt_frame_register is not called for the
>> no-longer-relevant frame types.
>
> I'd argue really just "weird and non-working", hardly dangerous. Even in
> the mac80211 design where we want to not let you intercept e.g. AUTH
> frames in client mode - if you did, then you'd just end up with a non-
> working interface. Not sure I see any "dangerous situation". Not really
> an all that important distinction though.
Fair enough, I'm happy to drop / reword this language. It seemed fishy
to me since the unregistration operation was not called at all, and the
driver does go to some lengths to set up the valid frame registration
types.
>
> Depending on the design, it may also just be that those registrations
> are *ignored*, because e.g. firmware intercepts the AUTH frame already,
> which would just (maybe) confuse userspace - but that seems unlikely
> since it switched interface type and has no real need for those frames
> then.
There might be corner cases where userspace gets confused and doesn't
update the frame registrations properly. For example, wpa_s/hostap does
not listen to SET_INTERFACE events that I can tell. So if some external
app sets the mode (particularly on a 'live' interface) then all kinds of
unexpected things might happen. This is one of the motivations for
restricting certain NL80211 commands to interface SOCKET_OWNER.
So really this patch is intended more as a hot-fix / backport to stable
to make sure the older kernels can deal with some of these situations.
>
>> The kernel currently relies on userspace apps to actually purge the
>> registrations themselves, e.g. by closing the nl80211 socket associated
>> with those frames. However, this requires multiple nl80211 sockets to
>> be open by the userspace app, and for userspace to be aware of all state
>> changes. This is not something that the kernel should rely on.
>
> I tend to agree with that the kernel shouldn't rely on it.
>
>> This commit adds a call to cfg80211_mlme_purge_registrations() to
>> forcefully remove any registrations left over prior to switching the
>> iftype.
>
> However, I do wonder if we should make this more transactional, and hang
> on to them if the type switching fails. We're not notifying userspace
> that the registrations have disappeared, so if type switching fails and
> it continues to work with the old type rather than throwing its hands up
> and quitting or something, it'd make a possibly bigger mess to just
> silently have removed them already.
I do like that idea, not sure how to go about implementing it though?
The failure case is a bit hard to deal with. Something like
NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE would help, particularly if
nl80211/cfg80211 actually checked it prior to doing anything (e.g.
disconnecting, etc). That would then take care of the majority of the
'typical' failure paths. I didn't add such checking in the other patch
set since I felt you might find it overly intrusive on userspace. But
maybe we really should do this?
So playing devil's advocate, another argument might be that by the time
we got here, we've already tore down a bunch of state. E.g.
disconnected the station, stopped AP, etc. So we've already
side-effected state in a bunch of ways, what's one more?
>
> I *think* it should be safe to just move this after the switching
> succeeds, since the switching can pretty much only be done at a point
> where nothing is happening on the interface anyway, though that might
> confuse the driver when the remove happens.
>
I would concur as that is what happens today. But should it?
> Also, perhaps it'd be better to actually hang on to those registrations
> that *are* still possible afterwards? But to not confuse the driver I
> guess that might require unregister/re-register to happen, all of which
> requires hanging on to the list and going through it after the type
> switch completed?
Yes, I had those exact thoughts as well.
It isn't currently clear to me if there are any guarantees on the driver
operation call sequence that cfg80211 provides. E.g. can the driver
expect rdev_change_virtual_intf to be called only once all the old
registrations are purged and the new registrations are performed after
the fact? Or should it expect things to just happen in any order?
>
> What do you think?
>
A big part of me thinks that just wiping the slate clean and having
userspace set it up from scratch isn't that much to ask and it might
want to do that anyway. It might (a big maybe?) also make the driver's
life easier if it can rely on certain guarantees from cfg80211. E.g.
that all invalid registrations are purged.
I have seen wpa_s perform a bunch of register commands which bounce off
with an -EALREADY. So it may already be erring on the side of caution
and assuming that it needs to reset the state fully? Not sure.
But if the kernel wants to be nice and spends some cycles figuring out
which frame registrations to keep and re-register them, that is also
fine with me.
Regards,
-Denis
^ permalink raw reply
* Re: [RFCv2 4/4] nl80211: Send large new_wiphy events
From: Denis Kenzior @ 2019-08-30 15:53 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
In-Reply-To: <2eb256e8a7be09f35dc4f0f3b61e0363691f41f0.camel@sipsolutions.net>
Hi Johannes,
On 8/30/19 5:14 AM, Johannes Berg wrote:
> On Fri, 2019-08-16 at 14:27 -0500, Denis Kenzior wrote:
>> Send large NEW_WIPHY events on a new multicast group so that clients
>> that can accept larger messages do not need to round-trip to the kernel
>> and perform extra filtered wiphy dumps.
>>
>> A new multicast group is introduced and the large message is sent before
>> the legacy message. This way clients that listen on both multicast
>> groups can ignore duplicate legacy messages if needed.
>
> Since I just did the digging, it seems that this would affect (old)
> applications with libnl up to 3.2.22, unless they changed the default
> recvmsg() buffer size.
>
Sorry, I'm not sure I understand. Are you saying new clients would try
to use old libnl and subscribe to this new multicast group for large
messages? Legacy clients shouldn't even see messages on this multicast
group since they would never subscribe to it.
> I think this is a pretty decent approach, but I'm slightly worried about
> hitting the new limits (16k) eventually. It seems far off now, but who
> knows what kind of data we'll add. HE is (and likely will be) adding
> quite a bit since it has everything for each interface type - something
> drivers have for the most part not implemented yet. That trend will only
> continue, as complexity in the spec doesn't seem to be going down.
>
Right, but the kernel will go up to 32k buffers if userspace read buffer
is that large. So I think we have quite some room to grow. On the
other hand, we probably should be vigilant that any new stuff added
tries to minimize message sizes whenever possible.
> And I don't really want to see "config3" a couple of years down the
> road...
>
Agreed.
> So can we at least mandate (document) that "config2" basically has no
> message limit, and you will use MSG_PEEK/handle MSG_TRUNC with it?
>
Yes, I will take care of that in v3.
> That way, we can later bump the 8192 even beyond 16k if needed, and not
> run into problems.
>
>> + if (cmd == NL80211_CMD_NEW_WIPHY) {
>> + state.large_message = true;
>> + alloc_size = 8192UL;
>> + } else
>> + alloc_size = NLMSG_DEFAULT_SIZE;
>> +
>
> nit: there should be braces on both branches
>
will fix
>> + if (nl80211_send_wiphy(rdev, cmd, msg, 0, 0, 0, &state) < 0) {
>> + nlmsg_free(msg);
>> + goto legacy;
>> + }
>
> I think that'd be worth a WARN_ON(), it should never happen that you
> actually run out of space, it means that the above wasn't big enough.
>
Yep
> Now, on the previous patches I actually thought that you could set
> "state->split" (and you should) and not need "state->large_message" in
> order to indicate that the sub-functions are allowed to create larger
> data - just keep filling the SKBs as much as possible for the dump.
>
> Here, it seems like we do need it. It might be possible to get away
> without it (by setting split here, and then having some special code to
> handle the case of it not getting to the end), but that doesn't seem
> worth it.
>
>> @@ -14763,6 +14787,8 @@ void nl80211_notify_iface(struct cfg80211_registered_device *rdev,
>> return;
>> }
>>
>> + genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
>> + NL80211_MCGRP_CONFIG2, GFP_KERNEL);
>
> Hmm. That seems only needed if you don't want to listen on "config" at
> all, but in the patch description you explicitly said that you send it
> on "config2" *before* "config" for compatibility reasons (which makes
> sense) - so what is it?
Well it can be both, depending on whether large messages can fail or
not. So one use case might be that a client detects whether the config2
multicast group exists. If so, then it only subscribes to it and that's it.
Another use case might be (if userspace is worried about losing large
messages) to subscribe to both groups. If it receives the large
message, it can ignore the one that comes on the legacy multicast group.
>
> I'm having a hard time seeing anyone get away with only listening on
> config2 since that'd basically require very recent (as of now future)
> kernel. Are you planning this for a world where you can ditch support
> for kernel<5.4 (or so)?
No, but there's nothing stopping the client in making the choice at
runtime depending on the genl family info it gets. E.g. by peeking into
CTRL_ATTR_MCAST_GROUPS.
Regards,
-Denis
^ permalink raw reply
* Re: [RFCv2 1/4] nl80211: Fix broken non-split wiphy dumps
From: Denis Kenzior @ 2019-08-30 16:32 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
In-Reply-To: <f7c98da178677cbb0cad3568f4ea4ab85171edd8.camel@sipsolutions.net>
Hi Johannes,
On 8/30/19 4:03 AM, Johannes Berg wrote:
> On Fri, 2019-08-16 at 14:27 -0500, Denis Kenzior wrote:
>> If a (legacy) client requested a wiphy dump but did not provide the
>> NL80211_ATTR_SPLIT_WIPHY_DUMP attribute, the dump was supposed to be
>> composed of purely non-split NEW_WIPHY messages, with 1 wiphy per
>> message. At least this was the intent after commit:
>> 3713b4e364ef ("nl80211: allow splitting wiphy information in dumps")
>>
>> However, in reality the non-split dumps were broken very shortly after.
>> Perhaps around commit:
>> fe1abafd942f ("nl80211: re-add channel width and extended capa advertising")
>
> Fun. I guess we updated all userspace quickly enough to not actually
> have any issues there. As far as I remember, nobody ever complained, so
> I guess people just updated their userspace.
>
> Given that it's been 6+ years, maybe we're better off just removing the
> whole non-split thing then, instead of fixing it. Seems even less likely
> now that somebody would run a 6+yo supplicant (from before its commit
> c30a4ab045ce ("nl80211: Fix mode settings with split wiphy dump")).
>
That would be my vote, given that we're probably one of a handful of
people in this world that understand that code path.
But... How would we handle non-dump versions of GET_WIPHY? To this day
I have dhcpcd issuing fun stuff like:
< Request: Get Wiphy (0x01) len 8 [ack]
0.374832
Interface Index: 59 (0x0000003b)
> OTOH, this is a simple fix, would removing the non-split mode result in
> any appreciable cleanups? Perhaps not, and we'd have to insert something
> instead to reject non-split and log a warning, or whatnot.
>
Getting rid of the legacy non-split case would simplify things. We
could also be a-lot smarter about how we split up the messages in order
to utilize buffer space more efficiently. I think you cover this in
your other replies, but I haven't processed those yet.
Regards,
-Denis
^ permalink raw reply
* Re: [PATCH 1/2] nl80211: Add NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE
From: Denis Kenzior @ 2019-08-30 16:55 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
In-Reply-To: <f5986ccd8ecdcc08d5c3e0d65f8bddef8b0af021.camel@sipsolutions.net>
Hi Johannes,
On 8/30/19 5:19 AM, Johannes Berg wrote:
> On Mon, 2019-08-26 at 11:26 -0500, Denis Kenzior wrote:
>>
>> + * Prior to Kernel 5.4, userspace applications should implement the
>> + * following behavior:
>
> I'm not sure mentioning the kernel version here does us any good? I
> mean, you really need to implement that behaviour regardless of kernel
> version, if NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE isn't set.
>
Agreed. I guess I just view nl80211.h as a sort of combination between
a uapi file and an actual manpage. And manpages do mention which kernel
version a certain feature/flag/whatever was added. Such info can be
useful in many ways, e.g. figuring out which kernel version might be
required for a certain piece of hardware, etc.
Another point where this might be useful is if the kernel starts
enforcing certain behavior that it didn't before. E.g. I mentioned this
in the purge thread that a lot of mode change failure cases could be
caught if the kernel checked this flag prior to doing anything.
I really leave this up to you if this is something you think is a good
idea or not.
>> + * @NL80211_EXT_FEATURE_LIVE_IFTYPE_CHANGE: This device supports switching
>> + * the IFTYPE of an interface without having to bring the device DOWN
>> + * first via RTNL. Exact semantics of this feature is driver
>> + * implementation dependent.
>
> That's not really nice.
Sorry. This came from some doc changes I have pending. I think I wrote
this after looking at some fullmac drivers and how they handle mode
changes and the wording reflects the exasperation I felt at the time.
Do you want to suggest some alternate wording? I think it is worth it
to have some fair warning in the docs stating that prior to version so
and so the semantics are completely driver dependent.
>
>> For mac80211, the following restrictions
>> + * apply:
>> + * - Only devices currently in IFTYPE AP, P2P_GO, P2P_CLIENT,
>> + * STATION, ADHOC and OCB can be switched.
>> + * - The target IFTYPE must be one of: AP, P2P_GO, P2P_CLIENT,
>> + * STATION, ADHOC or OCB.
>> + * Other drivers are expected to follow similar restrictions.
>
> Maybe we should instead have a "bitmask of interface types that can be
> switched while live" or something like that?
>
I'm fine with that, but this would only apply to newer kernels, no?
Don't we at least want to attempt to state what the rules are for older
ones?
An alternative might be to simply state what the restrictions are and
just enforce those at the cfg80211 level.
Regards,
-Denis
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox