diff mbox series

[RFC,v3,08/12] iwlwifi: dvm - EXT_KEY_ID A-MPDU API update

Message ID 20190210210620.31181-9-alexander@wetzel-home.de (mailing list archive)
State RFC
Delegated to: Johannes Berg
Headers show
Series Draft for Extended Key ID support | expand

Commit Message

Alexander Wetzel Feb. 10, 2019, 9:06 p.m. UTC
When using Extended Key ID mac80211 drops @IEEE80211_TX_CTL_AMPDU for
the last packet which can be added to a A-MPDU in preparation.

Don't throw a warning and just handle the frame as if
@IEEE80211_TX_CTL_AMPDU would have been set.

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

I cold not figure out so far how to make sure the card will not mix
A-MPDU frames. Looks like that is handled fully in the HW and I didn't
find any interface to influence it. (It even may work already, I have
some problems to capture A-MPDU frames with A-MPDU information intact
and the topic was pretty low on the list so far. After all it works...)

This patch is therefore basically just using aggregation when it's
enabled and ignores the key border signal from mac80211.

 drivers/net/wireless/intel/iwlwifi/dvm/tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Johannes Berg Feb. 15, 2019, 11:54 a.m. UTC | #1
On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote:
> When using Extended Key ID mac80211 drops @IEEE80211_TX_CTL_AMPDU for
> the last packet which can be added to a A-MPDU in preparation.
> 
> Don't throw a warning and just handle the frame as if
> @IEEE80211_TX_CTL_AMPDU would have been set.
> 
> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
> ---
> 
> I cold not figure out so far how to make sure the card will not mix
> A-MPDU frames. Looks like that is handled fully in the HW and I didn't
> find any interface to influence it. (It even may work already, I have
> some problems to capture A-MPDU frames with A-MPDU information intact
> and the topic was pretty low on the list so far. After all it works...)

You can't really do that, as far as I can tell, unfortunately. So this
might be better in a "compat on steroids" mode?

johannes
Alexander Wetzel Feb. 22, 2019, 9:15 p.m. UTC | #2
> On Sun, 2019-02-10 at 22:06 +0100, Alexander Wetzel wrote:
>> When using Extended Key ID mac80211 drops @IEEE80211_TX_CTL_AMPDU for
>> the last packet which can be added to a A-MPDU in preparation.
>>
>> Don't throw a warning and just handle the frame as if
>> @IEEE80211_TX_CTL_AMPDU would have been set.
>>
>> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
>> ---
>>
>> I cold not figure out so far how to make sure the card will not mix
>> A-MPDU frames. Looks like that is handled fully in the HW and I didn't
>> find any interface to influence it. (It even may work already, I have
>> some problems to capture A-MPDU frames with A-MPDU information intact
>> and the topic was pretty low on the list so far. After all it works...)
> 
> You can't really do that, as far as I can tell, unfortunately. So this
> might be better in a "compat on steroids" mode?

I hoped that the HW would already enforce the key border. But ok: The 
cards will still be able to use NATIVE mode. Only now the driver will 
have to request to disable Tx aggregation when finding a key border. 
Since Tx aggregation is disabled per default anyhow many users will not 
even see a difference.

(Off topic here, but I suspect the not reproducible problems leading to 
change the Tx aggregation default to off could also have been ptk0 rekey 
freezes. In that case we could now allow it again. But I've not found 
raw error reports and guess it's not worth the efforts/risks to change 
the default again.)

Alexander
Johannes Berg Feb. 22, 2019, 9:20 p.m. UTC | #3
On Fri, 2019-02-22 at 22:15 +0100, Alexander Wetzel wrote:

> I hoped that the HW would already enforce the key border. 

I don't think so.

> But ok: The 
> cards will still be able to use NATIVE mode. Only now the driver will 
> have to request to disable Tx aggregation when finding a key border. 

It doesn't really have to even completely disable it, it'd just have to
send and LQ_CMD with agg_params.agg_frame_cnt_limit = 1, temporarily,
until the relevant frames were transmitted (this is a global config, so
it'd have to track the key border on each TID, but that's not _too_
hard).

> Since Tx aggregation is disabled per default anyhow many users will not 
> even see a difference.

Good point, I forgot all about that.

> (Off topic here, but I suspect the not reproducible problems leading to 
> change the Tx aggregation default to off could also have been ptk0 rekey 
> freezes. In that case we could now allow it again. But I've not found 
> raw error reports and guess it's not worth the efforts/risks to change 
> the default again.)

I wouldn't be so sure - PTK rekeying should still be a pretty uncommon
configuration. But I also don't recall the exact decision making going
into this.

johannes
diff mbox series

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/tx.c b/drivers/net/wireless/intel/iwlwifi/dvm/tx.c
index 4ff323a3a4e5..478f8e1c3e52 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/tx.c
@@ -420,7 +420,7 @@  int iwlagn_tx_skb(struct iwl_priv *priv,
 		hdr->seq_ctrl |= cpu_to_le16(seq_number);
 		seq_number += 0x10;
 
-		if (info->flags & IEEE80211_TX_CTL_AMPDU)
+		if (tid_data->agg.state == IWL_AGG_ON)
 			is_agg = true;
 		is_data_qos = true;
 	}