[PATCH 1/4] mac80211_hwsim: Extended Key ID API update

[PATCH 1/4] mac80211_hwsim: Extended Key ID API update

[Alexander Wetzel]

Prepare hwsim Extended Key ID support for a mac80211 API change.

The mac80211 flag IEEE80211_HW_EXT_KEY_ID_NATIVE is being replaced by
NL80211_EXT_FEATURE_EXT_KEY_ID which only must be set by drivers when
they support HW crypto.

This reverts commit cfe7007a9b4cea9c4a0f7d4192c776c62f31869e.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---
 drivers/net/wireless/mac80211_hwsim.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index d396a33bbc9c..26cbb5b5d7cd 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2805,12 +2805,6 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
 	ieee80211_hw_set(hw, SIGNAL_DBM);
 	ieee80211_hw_set(hw, SUPPORTS_PS);
 	ieee80211_hw_set(hw, TDLS_WIDER_BW);
-
-	/* We only have SW crypto and only implement the A-MPDU API
-	 * (but don't really build A-MPDUs) so can have extended key
-	 * support
-	 */
-	ieee80211_hw_set(hw, EXT_KEY_ID_NATIVE);
 	if (rctbl)
 		ieee80211_hw_set(hw, SUPPORTS_RC_TABLE);
 	ieee80211_hw_set(hw, SUPPORTS_MULTI_BSSID);
-- 
2.22.0

[PATCH 2/4] mac80211: Simplify Extended Key ID API

[Alexander Wetzel]

1) Drop IEEE80211_HW_EXT_KEY_ID_NATIVE and let drivers directly set
   the NL80211_EXT_FEATURE_EXT_KEY_ID flag.

2) Drop IEEE80211_HW_NO_AMPDU_KEYBORDER_SUPPORT and simply assume all
   drivers are unable to handle A-MPDU key borders.

The new Extended Key ID API now requires all mac80211 drivers to set
NL80211_EXT_FEATURE_EXT_KEY_ID when they implement set_key() and can
handle Extended Key ID. For drivers not providing set_key() mac80211
itself enables Extended Key ID support, using the internal SW crypto
services.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---
Deciding to not merge the COMPAT Extended Key ID support also
invalidated the reasoning to have IEEE80211_HW_EXT_KEY_ID_NATIVE in the
first place. We can simple drop the flag and ask drivers to directly
set NL80211_EXT_FEATURE_EXT_KEY_ID with the current code.

IEEE80211_HW_NO_AMPDU_KEYBORDER_SUPPORT was intended to tell mac80211
that the driver is not checking the keyid when aggregating frames and is
only compatible with the IEEE 802.11 - 2016 standard when there are no
aggregation sessions running during rekey. But reverting the logic makes
more sense, the only driver able to set it for the foreseeable future is
hwsim. And for hwsim it's irrelevant if we stop A-MPDU or not, the
driver is never really aggregating frames.

Now it probably makes sense to not yet implement AMPDU_KEYBORDER_SUPPORT
and wait to see if we really have a need for that. So this patch stops
A-MPDU sessions every time when we rekey with Extended Key ID and is not
providing a API to the driver to prevent that.

But the next patch in the series is implementing the feature, allowing
you to merge it and have the API available today or simply skip it.

 include/net/mac80211.h |  8 --------
 net/mac80211/debugfs.c |  2 --
 net/mac80211/key.c     | 18 ++++++++----------
 net/mac80211/main.c    | 18 ++++++------------
 4 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d26da013f7c0..544dad54b11f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2268,12 +2268,6 @@ struct ieee80211_txq {
  * @IEEE80211_HW_SUPPORTS_ONLY_HE_MULTI_BSSID: Hardware supports multi BSSID
  *	only for HE APs. Applies if @IEEE80211_HW_SUPPORTS_MULTI_BSSID is set.
  *
- * @IEEE80211_HW_EXT_KEY_ID_NATIVE: Driver and hardware are supporting Extended
- *	Key ID and can handle two unicast keys per station for Rx and Tx.
- *
- * @IEEE80211_HW_NO_AMPDU_KEYBORDER_SUPPORT: The card/driver can't handle
- *	active Tx A-MPDU sessions with Extended Key IDs during rekey.
- *
  * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
  */
 enum ieee80211_hw_flags {
@@ -2325,8 +2319,6 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_TX_STATUS_NO_AMPDU_LEN,
 	IEEE80211_HW_SUPPORTS_MULTI_BSSID,
 	IEEE80211_HW_SUPPORTS_ONLY_HE_MULTI_BSSID,
-	IEEE80211_HW_EXT_KEY_ID_NATIVE,
-	IEEE80211_HW_NO_AMPDU_KEYBORDER_SUPPORT,
 
 	/* keep last, obviously */
 	NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 2e7f75938c51..47435f57e086 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -271,8 +271,6 @@ static const char *hw_flag_names[] = {
 	FLAG(TX_STATUS_NO_AMPDU_LEN),
 	FLAG(SUPPORTS_MULTI_BSSID),
 	FLAG(SUPPORTS_ONLY_HE_MULTI_BSSID),
-	FLAG(EXT_KEY_ID_NATIVE),
-	FLAG(NO_AMPDU_KEYBORDER_SUPPORT),
 #undef FLAG
 };
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index dd60f6428049..92c3affb0eb0 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -270,8 +270,7 @@ int ieee80211_set_tx_key(struct ieee80211_key *key)
 
 	sta->ptk_idx = key->conf.keyidx;
 
-	if (ieee80211_hw_check(&local->hw, NO_AMPDU_KEYBORDER_SUPPORT))
-		clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
+	clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
 	ieee80211_check_fast_xmit(sta);
 
 	return 0;
@@ -289,16 +288,15 @@ static void ieee80211_pairwise_rekey(struct ieee80211_key *old,
 	if (new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX) {
 		/* Extended Key ID key install, initial one or rekey */
 
-		if (sta->ptk_idx != INVALID_PTK_KEYIDX &&
-		    ieee80211_hw_check(&local->hw,
-				       NO_AMPDU_KEYBORDER_SUPPORT)) {
+		if (sta->ptk_idx != INVALID_PTK_KEYIDX) {
 			/* Aggregation Sessions with Extended Key ID must not
 			 * mix MPDUs with different keyIDs within one A-MPDU.
-			 * Tear down any running Tx aggregation and all new
-			 * Rx/Tx aggregation request during rekey if the driver
-			 * asks us to do so. (Blocking Tx only would be
-			 * sufficient but WLAN_STA_BLOCK_BA gets the job done
-			 * for the few ms we need it.)
+			 * Tear down running Tx aggregation sessions and block
+			 * new Rx/Tx aggregation requests during rekey to
+			 * ensure there are no A-MPDUs for the driver to
+			 * aggregate. (Blocking Tx only would be sufficient but
+			 * WLAN_STA_BLOCK_BA gets the job done for the few ms
+			 * we need it.)
 			 */
 			set_sta_flag(sta, WLAN_STA_BLOCK_BA);
 			mutex_lock(&sta->ampdu_mlme.mtx);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 85e416248753..3b8eb5d2ec7e 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1048,21 +1048,15 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 		}
 	}
 
-	/* Enable Extended Key IDs when driver allowed it, or when it
-	 * supports neither HW crypto nor A-MPDUs
+	/* Mac80211 and therefore all drivers using SW crypto only
+	 * are able to handle PTK rekeys and Extended Key ID.
 	 */
-	if ((!local->ops->set_key &&
-	     !ieee80211_hw_check(hw, AMPDU_AGGREGATION)) ||
-	    ieee80211_hw_check(&local->hw, EXT_KEY_ID_NATIVE))
-		wiphy_ext_feature_set(local->hw.wiphy,
-				      NL80211_EXT_FEATURE_EXT_KEY_ID);
-
-	/* Mac80211 and therefore all cards only using SW crypto are able to
-	 * handle PTK rekeys correctly
-	 */
-	if (!local->ops->set_key)
+	if (!local->ops->set_key) {
 		wiphy_ext_feature_set(local->hw.wiphy,
 				      NL80211_EXT_FEATURE_CAN_REPLACE_PTK0);
+		wiphy_ext_feature_set(local->hw.wiphy,
+				      NL80211_EXT_FEATURE_EXT_KEY_ID);
+	}
 
 	/*
 	 * Calculate scan IE length -- we need this to alloc
-- 
2.22.0

[PATCH 3/4] mac80211: AMPDU handling for rekeys with Extended Key ID

[Alexander Wetzel]

Extended Key ID allows A-MPDU sessions while rekeying as long as each
A-MPDU aggregates only MPDUs with one keyid together.

Drivers able to segregate MPDUs accordingly can tell mac80211 to not
stop A-MPDU sessions when rekeying by setting the new flag
IEEE80211_HW_AMPDU_KEYBORDER_SUPPORT.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---
 include/net/mac80211.h |  5 +++++
 net/mac80211/debugfs.c |  1 +
 net/mac80211/key.c     | 14 ++++++++------
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 544dad54b11f..1d6bb67ecb47 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2268,6 +2268,10 @@ struct ieee80211_txq {
  * @IEEE80211_HW_SUPPORTS_ONLY_HE_MULTI_BSSID: Hardware supports multi BSSID
  *	only for HE APs. Applies if @IEEE80211_HW_SUPPORTS_MULTI_BSSID is set.
  *
+ * @IEEE80211_HW_AMPDU_KEYBORDER_SUPPORT: The card and driver is only
+ *	aggregating MPDUs with the same keyid, allowing mac80211 to keep Tx
+ *	A-MPDU sessions active while rekeying with Extended Key ID.
+ *
  * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
  */
 enum ieee80211_hw_flags {
@@ -2319,6 +2323,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_TX_STATUS_NO_AMPDU_LEN,
 	IEEE80211_HW_SUPPORTS_MULTI_BSSID,
 	IEEE80211_HW_SUPPORTS_ONLY_HE_MULTI_BSSID,
+	IEEE80211_HW_AMPDU_KEYBORDER_SUPPORT,
 
 	/* keep last, obviously */
 	NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 47435f57e086..568b3b276931 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -271,6 +271,7 @@ static const char *hw_flag_names[] = {
 	FLAG(TX_STATUS_NO_AMPDU_LEN),
 	FLAG(SUPPORTS_MULTI_BSSID),
 	FLAG(SUPPORTS_ONLY_HE_MULTI_BSSID),
+	FLAG(AMPDU_KEYBORDER_SUPPORT),
 #undef FLAG
 };
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 92c3affb0eb0..7dfee848abac 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -270,7 +270,8 @@ int ieee80211_set_tx_key(struct ieee80211_key *key)
 
 	sta->ptk_idx = key->conf.keyidx;
 
-	clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
+	if (!ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT))
+		clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
 	ieee80211_check_fast_xmit(sta);
 
 	return 0;
@@ -288,15 +289,16 @@ static void ieee80211_pairwise_rekey(struct ieee80211_key *old,
 	if (new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX) {
 		/* Extended Key ID key install, initial one or rekey */
 
-		if (sta->ptk_idx != INVALID_PTK_KEYIDX) {
+		if (sta->ptk_idx != INVALID_PTK_KEYIDX &&
+		    !ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT)) {
 			/* Aggregation Sessions with Extended Key ID must not
 			 * mix MPDUs with different keyIDs within one A-MPDU.
 			 * Tear down running Tx aggregation sessions and block
 			 * new Rx/Tx aggregation requests during rekey to
-			 * ensure there are no A-MPDUs for the driver to
-			 * aggregate. (Blocking Tx only would be sufficient but
-			 * WLAN_STA_BLOCK_BA gets the job done for the few ms
-			 * we need it.)
+			 * ensure there are no A-MPDUs when the driver is not
+			 * supporting A-MPDU key borders. (Blocking Tx only
+			 * would be sufficient but WLAN_STA_BLOCK_BA gets the
+			 * job done for the few ms we need it.)
 			 */
 			set_sta_flag(sta, WLAN_STA_BLOCK_BA);
 			mutex_lock(&sta->ampdu_mlme.mtx);
-- 
2.22.0

[PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Alexander Wetzel]

All iwlwifi cards are able to handle multiple keyids per STA and are
therefore fully compatible with the Extended Key ID implementation
provided by mac80211.

Allow Extended Key ID to be used for all mvm and dvm cards.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

This is basically the v2 patch of https://patchwork.kernel.org/patch/10931879/
which Luca still has in his review queue. It just uses the new proposed
simplified Extended Key ID API from this patch series instead.

Merging (parts) of this series will of course break the older patch
still queued to Luca, so this may need some coordination.

 drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c | 1 +
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c
index 6c170636110a..ac88c19f4f18 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/mac80211.c
@@ -200,6 +200,7 @@ int iwlagn_mac_setup_register(struct iwl_priv *priv,
 	iwl_leds_init(priv);
 
 	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
+	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_EXT_KEY_ID);
 
 	ret = ieee80211_register_hw(priv->hw);
 	if (ret) {
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index fdbabca0280e..c752fe6970e3 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -599,6 +599,7 @@ int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm)
 
 	hw->wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
 	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_VHT_IBSS);
+	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_EXT_KEY_ID);
 	hw->wiphy->features |= NL80211_FEATURE_HT_IBSS;
 
 	hw->wiphy->regulatory_flags |= REGULATORY_ENABLE_RELAX_NO_IR;
-- 
2.22.0

Re: [PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Luca Coelho]

On Sat, 2019-06-29 at 21:50 +0200, Alexander Wetzel wrote:
> All iwlwifi cards are able to handle multiple keyids per STA and are
> therefore fully compatible with the Extended Key ID implementation
> provided by mac80211.
> 
> Allow Extended Key ID to be used for all mvm and dvm cards.
> 
> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
> ---
> 
> This is basically the v2 patch of https://patchwork.kernel.org/patch/10931879/
> which Luca still has in his review queue. It just uses the new proposed
> simplified Extended Key ID API from this patch series instead.
> 
> Merging (parts) of this series will of course break the older patch
> still queued to Luca, so this may need some coordination.

Thanks for your patch! I've dropped the old version and will wait until
Johannes merges the mac80211 part (and it reaches wireless-drivers-
next), so I can apply this.

--
Cheers,
Luca.

Re: [PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Alexander Wetzel]

> All iwlwifi cards are able to handle multiple keyids per STA and are
> therefore fully compatible with the Extended Key ID implementation
> provided by mac80211.

I just tried Extended Key ID with a AX200 card and it really looks like 
it's incompatible:-(

The card is starting to use the PTK key immediately after installation, 
encrypting EAPOL #3 with the new (still Rx only!) key.

Digging around in the driver code it looks like we do not even pass the 
key information any longer to the card: iwl_mvm_set_tx_params() is 
bypassing iwl_mvm_set_tx_cmd_crypto() completely when we use the "new tx 
API". So all cards setting "use_tfh" to true are now incompatible.

Therefore it looks like that all cards starting with the 22000 series 
can't be used with Extended Key ID any longer.

Is there a way to hand over the key information within the new API or is 
the way forward to block Extended Key ID when the "new tx API" is being 
used?

The card is fine with using keyid 1 for unicast keys. But it looks like 
it assumes that a new key install also tells it to use the new key 
immediately... Still digging around but pretty sure that's happening now.

Alexander

Re: [PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Johannes Berg]

On Sat, 2019-08-17 at 10:31 +0200, Alexander Wetzel wrote:
> > All iwlwifi cards are able to handle multiple keyids per STA and are
> > therefore fully compatible with the Extended Key ID implementation
> > provided by mac80211.
> 
> I just tried Extended Key ID with a AX200 card and it really looks like 
> it's incompatible:-(

Hmm.

> The card is starting to use the PTK key immediately after installation, 
> encrypting EAPOL #3 with the new (still Rx only!) key.

Right. This wasn't considered, I guess.

> Digging around in the driver code it looks like we do not even pass the 
> key information any longer to the card: iwl_mvm_set_tx_params() is 
> bypassing iwl_mvm_set_tx_cmd_crypto() completely when we use the "new tx 
> API". So all cards setting "use_tfh" to true are now incompatible.
> 
> Therefore it looks like that all cards starting with the 22000 series 
> can't be used with Extended Key ID any longer.
> 
> Is there a way to hand over the key information within the new API or is 
> the way forward to block Extended Key ID when the "new tx API" is being 
> used?

Not right now, but I think it could be fixed.

> The card is fine with using keyid 1 for unicast keys. But it looks like 
> it assumes that a new key install also tells it to use the new key 
> immediately... Still digging around but pretty sure that's happening now.

Right.

For now I guess we have to disable it with the new TX API (which is
really what it depends on), we can try to fix the firmware later.

johannes

Re: [PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Alexander Wetzel]

Am 19.08.19 um 11:43 schrieb Johannes Berg:
> On Sat, 2019-08-17 at 10:31 +0200, Alexander Wetzel wrote:
>>> All iwlwifi cards are able to handle multiple keyids per STA and are
>>> therefore fully compatible with the Extended Key ID implementation
>>> provided by mac80211.
>>
>> I just tried Extended Key ID with a AX200 card and it really looks like
>> it's incompatible:-(
> 
> Hmm.
> 
>> The card is starting to use the PTK key immediately after installation,
>> encrypting EAPOL #3 with the new (still Rx only!) key.
> 
> Right. This wasn't considered, I guess.
> 
>> Digging around in the driver code it looks like we do not even pass the
>> key information any longer to the card: iwl_mvm_set_tx_params() is
>> bypassing iwl_mvm_set_tx_cmd_crypto() completely when we use the "new tx
>> API". So all cards setting "use_tfh" to true are now incompatible.
>>
>> Therefore it looks like that all cards starting with the 22000 series
>> can't be used with Extended Key ID any longer.
>>
>> Is there a way to hand over the key information within the new API or is
>> the way forward to block Extended Key ID when the "new tx API" is being
>> used?
> 
> Not right now, but I think it could be fixed.

That would be great!

We may also get away by adding only means to pass the keyid of the MPDU 
(zero or one) to the HW. That could be done quite simple, I think:

We could add two new flags, e.g. IWL_TX_FLAGS_ENCRYPT_ID_0 and 
IWL_TX_FLAGS_ENCRYPT_ID_1 to avoid the need to change the structures 
iwl_tx_cmd_gen2 and iwl_tx_cmd_gen3.
When the firmware would check and use the key referenced by the STA + 
flag-id prior to the "last installed" key that should be sufficient.
By still using the last installed key without any of the new flags set 
we also would remain backward compatible.

If you have any experimental firmware to test I'm happy to do so:-)
Till then I'm back using older iwlwifi cards.

> 
>> The card is fine with using keyid 1 for unicast keys. But it looks like
>> it assumes that a new key install also tells it to use the new key
>> immediately... Still digging around but pretty sure that's happening now.
> 
> Right.
> 
> For now I guess we have to disable it with the new TX API (which is
> really what it depends on), we can try to fix the firmware later.

Ok. I'll update the iwlwifi Extended key ID support patch accordingly.

Alexander

Re: [PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Johannes Berg]

On Mon, 2019-08-19 at 17:52 +0200, Alexander Wetzel wrote:

> We may also get away by adding only means to pass the keyid of the MPDU 
> (zero or one) to the HW. That could be done quite simple, I think:
> 
> We could add two new flags, e.g. IWL_TX_FLAGS_ENCRYPT_ID_0 and 
> IWL_TX_FLAGS_ENCRYPT_ID_1 to avoid the need to change the structures 
> iwl_tx_cmd_gen2 and iwl_tx_cmd_gen3.
> When the firmware would check and use the key referenced by the STA + 
> flag-id prior to the "last installed" key that should be sufficient.
> By still using the last installed key without any of the new flags set 
> we also would remain backward compatible.
> 
> If you have any experimental firmware to test I'm happy to do so:-)
> Till then I'm back using older iwlwifi cards.

I'm not convinced that we can change the TX API at all, I suspect we
have to go detect it as we saw in the other patch. If we do actually
have the ability to change the TX API it might be simpler overall, but
anyway, I'd have to go look at how this is all implemented before I
comment further. Doesn't seem like an intractable problem, the only
question is if we get to spend time on it :)

johannes

Re: [PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Alexander Wetzel]

Am 19.08.19 um 22:23 schrieb Johannes Berg:
> On Mon, 2019-08-19 at 17:52 +0200, Alexander Wetzel wrote:
> 
>> We may also get away by adding only means to pass the keyid of the MPDU
>> (zero or one) to the HW. That could be done quite simple, I think:
>>
>> We could add two new flags, e.g. IWL_TX_FLAGS_ENCRYPT_ID_0 and
>> IWL_TX_FLAGS_ENCRYPT_ID_1 to avoid the need to change the structures
>> iwl_tx_cmd_gen2 and iwl_tx_cmd_gen3.
>> When the firmware would check and use the key referenced by the STA +
>> flag-id prior to the "last installed" key that should be sufficient.
>> By still using the last installed key without any of the new flags set
>> we also would remain backward compatible.
>>
>> If you have any experimental firmware to test I'm happy to do so:-)
>> Till then I'm back using older iwlwifi cards.
> 
> I'm not convinced that we can change the TX API at all, I suspect we
> have to go detect it as we saw in the other patch. If we do actually
> have the ability to change the TX API it might be simpler overall, but
> anyway, I'd have to go look at how this is all implemented before I
> comment further. Doesn't seem like an intractable problem, the only
> question is if we get to spend time on it :)

You are thinking about keeping the tx API untouched and modify the key 
install logic?
Just prevent the firmware to activate a key for Tx when it's installed 
and notify the firmware by some means when the key can be used for Tx 
and then switch everything to the new key?

I guess there is no practical way I can get access to the firmware code, 
correct? For me it sounds harder than the optional flag extension I had 
in mind for the new tx API.

After all one of the existing flags can already suppress the encryption.
Checking for the presence of two optional flags and use these to select 
a different key sounded not very hard. But then I literally know nothing 
about that and if the card/firmware has some fundamental issue handling 
two unicast keys the picture changes of course...

So let's wait and see what you can turn up. Till then we have more than 
enough other cards supporting Extended Key ID:-)

Alexander

Re: [PATCH 4/4] iwlwifi: Enable Extended Key ID for mvm and dvm

[Johannes Berg]

Hi,

> You are thinking about keeping the tx API untouched and modify the key 
> install logic?
> Just prevent the firmware to activate a key for Tx when it's installed 
> and notify the firmware by some means when the key can be used for Tx 
> and then switch everything to the new key?

Something like that, yes.

> I guess there is no practical way I can get access to the firmware code, 
> correct? 

There isn't, yeah.

> For me it sounds harder than the optional flag extension I had 
> in mind for the new tx API.

Much more of the TX path is actually wired into the hardware, rather
than being software. I'm not sure how much of this (key selection)
really is though.

> So let's wait and see what you can turn up. Till then we have more than 
> enough other cards supporting Extended Key ID:-)

Yeah. I'll take a look, but I can't promise right now to work on it high
priority.

johannes