[PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.

Michal Kazior michal.kazior at tieto.com
Tue Jun 2 22:04:23 PDT 2015


On 2 June 2015 at 20:21, Liu CF/TW <cfliu.tw at gmail.com> wrote:
> On Tue, Jun 2, 2015 at 12:17 AM, Michal Kazior <michal.kazior at tieto.com> wrote:
>> On 1 June 2015 at 21:44, David Liu <cfliu.tw at gmail.com> wrote:
>> [...]
>>> --- a/drivers/net/wireless/ath/ath10k/core.h
>>> +++ b/drivers/net/wireless/ath/ath10k/core.h
>>> @@ -91,6 +91,7 @@ struct ath10k_skb_cb {
>>>                 u8 tid;
>>>                 u16 freq;
>>>                 bool is_offchan;
>>> +               bool nohwcrypt;
>>>                 struct ath10k_htt_txbuf *txbuf;
>>>                 u32 txbuf_paddr;
>>>         } __packed htt;
>>> @@ -349,6 +350,7 @@ struct ath10k_vif {
>>>         } u;
>>>
>>>         bool use_cts_prot;
>>> +       bool nohwcrypt;
>>
>> So this is a bit confusing. This is used only for tx policy only,
>> right? It should be named accordingly then. The other nohwcrypt in
>> skb_cb pretty much implies Tx already.
>>
>
> No, it's also for Rx.
> In this patch, in mac.c, if arvif->nohwcrypt is set, ath10k_send_key()
> won't install the actual key to hardware, but a clear key is
> installed. This makes all matching STA's encrypted frame passthrough
> HW and mac80211 can take care of decryption later in host.
> (in patch V2 I will take care of the IEEE80211_HW_SW_CRYPTO_CONTROL setting)

Ah, you're right. I've missed that somehow.


>> [...]
>>> @@ -484,6 +491,12 @@ enum ath10k_dev_flags {
>>>          * waiters should immediately cancel instead of waiting for a time out.
>>>          */
>>>         ATH10K_FLAG_CRASH_FLUSH,
>>> +
>>> +       /* Use Raw mode for Tx and Rx */
>>> +       ATH10K_RAW_MODE,
>>> +
>>> +       /* Disable HW crypto engine */
>>> +       ATH10K_HW_CRYPTO_DISABLED,
>>
>> You're missing the _FLAG prefix. Also this isn't documented well
>> enough. RAW_MODE implies sw crypto rx, no? Perhaps a more fine grained
>> approach would be better:
>
> Thanks. Will fix.
>
> Raw mode doesn't imply SW crypto only.
> Indeed Raw mode is the only way to get sw crypto support, but it also
> supports HW crypto.
> I just tested both HW/SW crypto cases working fine in raw mode.
>
> So my plan for the new cryptmode parameter has 3 values:
>
>        0    Use HW crypto engine only. This uses native WiFi mode Tx/Rx encap
>
>        1    Use SW crypto engine only. This uses raw mode Tx/Rx encap
>
>        2    Supports both SW & HW crypto engine. This uses raw mode Tx/Rx encap.
>
> Once this patch is in, I guess people would only use cryptmode=0 or 1.
> For cryptmode=2, I have subsequent changes to allow per BSS based
> control of HW/SW crypto selection.
> Plan is to make make arvif->nohwcrypt configurable via debugfs or
> nl80211 (subject to review feedback)
>
>>
>>  ATH10K_FLAG_RAW_TX,
>>  ATH10K_FLAG_RAW_RX,
>>  ATH10K_FLAG_SW_TX_CRYPTO,
>>  ATH10K_FLAG_SW_RX_CRYPTO,
>>
>> Obviously not all combinations are valid/doable but I think this will
>> make the code look more obvious.
>>
>
> That would be too many flags and too complex.
> I'd suggest keep the proposed ATH10K_RAW_MODE and ATH10K_HW_CRYPTO_DISABLED.
> Let's make Tx/Rx HW crypto always both enabled or both disabled AFA
> driver is concerned.

Okay. This suggestion was based on my incorrect interpretation of your
patch wrt arvif->nohwcrypt.


>>> diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
>>> index 85cca29..37fd2f83 100644
>>> --- a/drivers/net/wireless/ath/ath10k/hw.h
>>> +++ b/drivers/net/wireless/ath/ath10k/hw.h
>>> @@ -296,7 +296,7 @@ enum ath10k_hw_rate_cck {
>>>  #define TARGET_10X_RX_SKIP_DEFRAG_TIMEOUT_DUP_DETECTION_CHECK 1
>>>  #define TARGET_10X_VOW_CONFIG                  0
>>>  #define TARGET_10X_NUM_MSDU_DESC               (1024 + 400)
>>> -#define TARGET_10X_MAX_FRAG_ENTRIES            0
>>> +#define TARGET_10X_MAX_FRAG_ENTRIES            10
>>
>> This is probably enough at "2" (ath10k doesn't send more than 2 tx
>> fragments now). I assume fw crashes with raw tx if this isn't fixed,
>> correct?
>>
>> Sidenote: I guess TARGET_MAX_FRAG_ENTRIES could be fixed as well. It
>> might make sense for QCA61X4 hw2.1 which still uses the old Rx
>> indication event and might be able to do raw txrx + swcrypto. But I'm
>> a bit reluctant to change this without any testing.
>>
>
> Sure. I change it to 10 because the document I got from QCA says so.
> Since this is a global setting, I will remove this and keep it =0 for
> now so it doesn't affect existing HW based datapath.

Sure. I recall 10.x on QCA988X isn't really picky on this value.
QCA61X4 with wmi-tlv on the other hand needs an adequate value for
this configuration parameter or it'll crash horribly.


> Per QCA, the main issue not changing this would be SW crypto then
> won't be able to handle large Rx AMSDU.
> When HW is not doing Rx decryption, the whole AMSDU needs to be DMA to
> host for SW based decryption & AMSDU subframe deaggregation.

Hmm.. From what I know this setting refers to the max number of Tx
fragments that can be submitted via HTT TX_FRM command eventually to
HW MAC, not Rx fragments.


Michał



More information about the ath10k mailing list