[PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.
This change supports bypassing HW crypto engine by enabling raw Rx/Tx mode.
This will enable use cases such as sw crypto and raw tx injection.
This change introduces a new module param 'cryptmode'.
cryptmode:
0Use HW crypto engine. Use native WiFi mode Tx/Rx encap
1Use SW crypto engine. Use raw mode Tx/Rx encap
2Supports both SW & HW crypto engine. Use raw mode Tx/Rx encap.
Testing: used QCA988x hw 2.0 with 10.2 firmware.
ath10k
cryptmode
param Testing Status
----
0 HW CCMP/TKIP tested ok.
Note: HW crypto bypass not supported by ath10k hw in native WiFi
mode.
1 - mac80211 SW crypto tested ok. (Req. mac80211 change)
- raw Tx frame injection tested ok.
2 HW CCMP/TKIP tested ok.
- BSS 1 tested HW CCMP/TKIP ok.
- BSS 2 can bypass HW engine.
- Tested mac80211 cam do SW crypto. (Req. mac80211 change)
- Tested raw Tx encrypted frame injection.
Signed-off-by: David Liu
---
drivers/net/wireless/ath/ath10k/core.c | 17 +
drivers/net/wireless/ath/ath10k/core.h | 24 -
drivers/net/wireless/ath/ath10k/htt_rx.c | 3 +-
drivers/net/wireless/ath/ath10k/htt_tx.c | 3 ++
drivers/net/wireless/ath/ath10k/hw.h | 2 +-
drivers/net/wireless/ath/ath10k/mac.c| 61 +++-
drivers/net/wireless/ath/ath10k/wmi.c| 6 ++--
7 files changed, 102 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 59496a9..e91fc85 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -31,16 +31,19 @@
#include "wmi-ops.h"
unsigned int ath10k_debug_mask;
+bool ath10k_cryptmode_param;
static bool uart_print;
static bool skip_otp;
module_param_named(debug_mask, ath10k_debug_mask, uint, 0644);
+module_param_named(cryptmode, ath10k_cryptmode_param, bool, 0644);
module_param(uart_print, bool, 0644);
module_param(skip_otp, bool, 0644);
MODULE_PARM_DESC(debug_mask, "Debugging mask");
MODULE_PARM_DESC(uart_print, "Uart target debugging");
MODULE_PARM_DESC(skip_otp, "Skip otp failure for calibration in testmode");
+MODULE_PARM_DESC(cryptmode, "Crypto mode - 0: HW only, 1: SW only, 2:Both");
static const struct ath10k_hw_params ath10k_hw_params_list[] = {
{
@@ -991,6 +994,20 @@ static int ath10k_core_init_firmware_features(struct
ath10k *ar)
return -EINVAL;
}
+ if (ath10k_cryptmode_param == ATH10K_CRYPT_MODE_HW) {
+ clear_bit(ATH10K_RAW_MODE, &ar->dev_flags);
+ clear_bit(ATH10K_HW_CRYPTO_DISABLED, &ar->dev_flags);
+ } else {
+ if (!test_bit(ATH10K_FW_FEATURE_RAW_MODE_SUPPORT,
+ ar->fw_features)) {
+ ath10k_err(ar, "cryptmode > 0 requires raw mode support
from firmware");
+ return -EINVAL;
+ }
+ set_bit(ATH10K_RAW_MODE, &ar->dev_flags);
+ if (ath10k_cryptmode_param == ATH10K_CRYPT_MODE_SW)
+ set_bit(ATH10K_HW_CRYPTO_DISABLED, &ar->dev_flags);
+ }
+
/* Backwards compatibility for firmwares without
* ATH10K_FW_IE_WMI_OP_VERSION.
*/
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 45f9603..13799c8 100644
--- 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;
int num_legacy_stations;
int txpower;
struct wmi_wmm_params_all_arg wmm_params;
@@ -461,7 +463,12 @@ enum ath10k_fw_features {
ATH10K_FW_FEATURE_WOWLAN_SUPPORT = 6,
/* Don't trust error code from otp.bin */
- ATH10K_FW_FEATURE_IGNORE_OTP_RESULT,
+ ATH10K_FW_FEATURE_IGNORE_OTP_RESULT = 7,
+
+ /* Raw mode support. If supported, FW supports receiving and trasmitting
+* frames in raw mode
+*/
+ ATH10K_FW_FEATURE_RAW_MODE_SUPPORT = 8,
/* Some firmware revisions pad 4th hw address to 4 byte boundary making
* it 8 bytes long in Native Wifi Rx decap.
@@ -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_DISABL
Re: [PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.
Typo: [PATH 1/2] should be just [PATCH]. This is the only ath10k
change in this patch.
On Mon, Jun 1, 2015 at 12:44 PM, David Liu wrote:
> This change supports bypassing HW crypto engine by enabling raw Rx/Tx
> mode.
> This will enable use cases such as sw crypto and raw tx injection.
> This change introduces a new module param 'cryptmode'.
>
> cryptmode:
>
>0Use HW crypto engine. Use native WiFi mode Tx/Rx encap
>
>1Use SW crypto engine. Use raw mode Tx/Rx encap
>
>2Supports both SW & HW crypto engine. Use raw mode Tx/Rx encap.
>
> Testing: used QCA988x hw 2.0 with 10.2 firmware.
>
> ath10k
> cryptmode
> param Testing Status
> ----
>0 HW CCMP/TKIP tested ok.
>Note: HW crypto bypass not supported by ath10k hw in native
> WiFi
>mode.
>
>1 - mac80211 SW crypto tested ok. (Req. mac80211 change)
>- raw Tx frame injection tested ok.
>
>2 HW CCMP/TKIP tested ok.
>- BSS 1 tested HW CCMP/TKIP ok.
>- BSS 2 can bypass HW engine.
> - Tested mac80211 cam do SW crypto. (Req. mac80211 change)
> - Tested raw Tx encrypted frame injection.
>
> Signed-off-by: David Liu
> ---
> drivers/net/wireless/ath/ath10k/core.c | 17 +
> drivers/net/wireless/ath/ath10k/core.h | 24 -
> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 3 ++
> drivers/net/wireless/ath/ath10k/hw.h | 2 +-
> drivers/net/wireless/ath/ath10k/mac.c| 61
> +++-
> drivers/net/wireless/ath/ath10k/wmi.c| 6 ++--
> 7 files changed, 102 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c
> b/drivers/net/wireless/ath/ath10k/core.c
> index 59496a9..e91fc85 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -31,16 +31,19 @@
> #include "wmi-ops.h"
>
> unsigned int ath10k_debug_mask;
> +bool ath10k_cryptmode_param;
> static bool uart_print;
> static bool skip_otp;
>
> module_param_named(debug_mask, ath10k_debug_mask, uint, 0644);
> +module_param_named(cryptmode, ath10k_cryptmode_param, bool, 0644);
> module_param(uart_print, bool, 0644);
> module_param(skip_otp, bool, 0644);
>
> MODULE_PARM_DESC(debug_mask, "Debugging mask");
> MODULE_PARM_DESC(uart_print, "Uart target debugging");
> MODULE_PARM_DESC(skip_otp, "Skip otp failure for calibration in testmode");
> +MODULE_PARM_DESC(cryptmode, "Crypto mode - 0: HW only, 1: SW only, 2:Both");
>
> static const struct ath10k_hw_params ath10k_hw_params_list[] = {
> {
> @@ -991,6 +994,20 @@ static int ath10k_core_init_firmware_features(struct
> ath10k *ar)
> return -EINVAL;
> }
>
> + if (ath10k_cryptmode_param == ATH10K_CRYPT_MODE_HW) {
> + clear_bit(ATH10K_RAW_MODE, &ar->dev_flags);
> + clear_bit(ATH10K_HW_CRYPTO_DISABLED, &ar->dev_flags);
> + } else {
> + if (!test_bit(ATH10K_FW_FEATURE_RAW_MODE_SUPPORT,
> + ar->fw_features)) {
> + ath10k_err(ar, "cryptmode > 0 requires raw mode
> support from firmware");
> + return -EINVAL;
> + }
> + set_bit(ATH10K_RAW_MODE, &ar->dev_flags);
> + if (ath10k_cryptmode_param == ATH10K_CRYPT_MODE_SW)
> + set_bit(ATH10K_HW_CRYPTO_DISABLED, &ar->dev_flags);
> + }
> +
> /* Backwards compatibility for firmwares without
> * ATH10K_FW_IE_WMI_OP_VERSION.
> */
> diff --git a/drivers/net/wireless/ath/ath10k/core.h
> b/drivers/net/wireless/ath/ath10k/core.h
> index 45f9603..13799c8 100644
> --- 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;
> int num_legacy_stations;
> int txpower;
> struct wmi_wmm_params_all_arg wmm_params;
> @@ -461,7 +463,12 @@ enum ath10k_fw_features {
> ATH10K_FW_FEATURE_WOWLAN_SUPPORT = 6,
>
> /* Don't trust error code from otp.bin */
> - ATH10K_FW_FEATURE_IGNORE_OTP_RESULT,
> + ATH10K_FW_FEATURE_IGNORE_OTP_RESULT = 7,
> +
> + /* Raw mode support. If supported, FW supports receiving and
> trasmitting
> +* frames in raw mode
> +*/
> + ATH10K_FW_FEATURE_RAW_MODE_SUPPORT = 8,
>
> /* Some firmware revisions pad 4th hw address to 4 byte boun
Re: [PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.
This patch still requires some more works. Please ignore.
I will send another one to address comments from linux-wireless ML.
Thanks
David
On Mon, Jun 1, 2015 at 12:47 PM, Liu CF/TW wrote:
> Typo: [PATH 1/2] should be just [PATCH]. This is the only ath10k
> change in this patch.
>
>
> On Mon, Jun 1, 2015 at 12:44 PM, David Liu wrote:
>> This change supports bypassing HW crypto engine by enabling raw Rx/Tx
>> mode.
>> This will enable use cases such as sw crypto and raw tx injection.
>> This change introduces a new module param 'cryptmode'.
>>
>> cryptmode:
>>
>>0Use HW crypto engine. Use native WiFi mode Tx/Rx encap
>>
>>1Use SW crypto engine. Use raw mode Tx/Rx encap
>>
>>2Supports both SW & HW crypto engine. Use raw mode Tx/Rx encap.
>>
>> Testing: used QCA988x hw 2.0 with 10.2 firmware.
>>
>> ath10k
>> cryptmode
>> param Testing Status
>> ----
>>0 HW CCMP/TKIP tested ok.
>>Note: HW crypto bypass not supported by ath10k hw in native
>> WiFi
>>mode.
>>
>>1 - mac80211 SW crypto tested ok. (Req. mac80211 change)
>>- raw Tx frame injection tested ok.
>>
>>2 HW CCMP/TKIP tested ok.
>>- BSS 1 tested HW CCMP/TKIP ok.
>>- BSS 2 can bypass HW engine.
>> - Tested mac80211 cam do SW crypto. (Req. mac80211 change)
>> - Tested raw Tx encrypted frame injection.
>>
>> Signed-off-by: David Liu
>> ---
>> drivers/net/wireless/ath/ath10k/core.c | 17 +
>> drivers/net/wireless/ath/ath10k/core.h | 24 -
>> drivers/net/wireless/ath/ath10k/htt_rx.c | 3 +-
>> drivers/net/wireless/ath/ath10k/htt_tx.c | 3 ++
>> drivers/net/wireless/ath/ath10k/hw.h | 2 +-
>> drivers/net/wireless/ath/ath10k/mac.c| 61
>> +++-
>> drivers/net/wireless/ath/ath10k/wmi.c| 6 ++--
>> 7 files changed, 102 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/core.c
>> b/drivers/net/wireless/ath/ath10k/core.c
>> index 59496a9..e91fc85 100644
>> --- a/drivers/net/wireless/ath/ath10k/core.c
>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>> @@ -31,16 +31,19 @@
>> #include "wmi-ops.h"
>>
>> unsigned int ath10k_debug_mask;
>> +bool ath10k_cryptmode_param;
>> static bool uart_print;
>> static bool skip_otp;
>>
>> module_param_named(debug_mask, ath10k_debug_mask, uint, 0644);
>> +module_param_named(cryptmode, ath10k_cryptmode_param, bool, 0644);
>> module_param(uart_print, bool, 0644);
>> module_param(skip_otp, bool, 0644);
>>
>> MODULE_PARM_DESC(debug_mask, "Debugging mask");
>> MODULE_PARM_DESC(uart_print, "Uart target debugging");
>> MODULE_PARM_DESC(skip_otp, "Skip otp failure for calibration in testmode");
>> +MODULE_PARM_DESC(cryptmode, "Crypto mode - 0: HW only, 1: SW only, 2:Both");
>>
>> static const struct ath10k_hw_params ath10k_hw_params_list[] = {
>> {
>> @@ -991,6 +994,20 @@ static int ath10k_core_init_firmware_features(struct
>> ath10k *ar)
>> return -EINVAL;
>> }
>>
>> + if (ath10k_cryptmode_param == ATH10K_CRYPT_MODE_HW) {
>> + clear_bit(ATH10K_RAW_MODE, &ar->dev_flags);
>> + clear_bit(ATH10K_HW_CRYPTO_DISABLED, &ar->dev_flags);
>> + } else {
>> + if (!test_bit(ATH10K_FW_FEATURE_RAW_MODE_SUPPORT,
>> + ar->fw_features)) {
>> + ath10k_err(ar, "cryptmode > 0 requires raw mode
>> support from firmware");
>> + return -EINVAL;
>> + }
>> + set_bit(ATH10K_RAW_MODE, &ar->dev_flags);
>> + if (ath10k_cryptmode_param == ATH10K_CRYPT_MODE_SW)
>> + set_bit(ATH10K_HW_CRYPTO_DISABLED, &ar->dev_flags);
>> + }
>> +
>> /* Backwards compatibility for firmwares without
>> * ATH10K_FW_IE_WMI_OP_VERSION.
>> */
>> diff --git a/drivers/net/wireless/ath/ath10k/core.h
>> b/drivers/net/wireless/ath/ath10k/core.h
>> index 45f9603..13799c8 100644
>> --- 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;
>> int num_legacy_stations;
>> int txpower;
>> struct wmi_wmm_params_all_arg wmm_params;
>> @@ -461,7 +463,12 @@ enum ath10k_fw_features {
>> ATH10K_FW_FEATURE_WOWLAN_SUPPORT = 6,
>>
>> /* Don't trust error code from otp.bin */
>> - ATH10K_FW_FEATURE_IGNORE_
Re: [PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.
On 1 June 2015 at 21:44, David Liu 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.
[...]
> @@ -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:
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.
> };
>
> enum ath10k_cal_mode {
> @@ -492,6 +505,15 @@ enum ath10k_cal_mode {
> ATH10K_CAL_MODE_DT,
> };
>
> +enum ath10k_crypt_mode {
> + /* Use HW crypto engine only */
> + ATH10K_CRYPT_MODE_HW,
> + /* HW SW crypto engine only (ie. HW crypto engine disabled) */
> + ATH10K_CRYPT_MODE_SW,
> + /* Both SW & HW crypto engine supported */
> + ATH10K_CRYPT_MODE_HW_SW,
I don't think this is clear enough (and the comments don't help at all):
ATH10K_CRYPT_MODE_HW,
ATH10K_CRYPT_MODE_SW,
ATH10K_CRYPT_MODE_SW_RX_HW_TX,
It would also be nice to have some sort of indication in driver
logs/dmesg (without any debug_masks) as to what crypto mode driver
uses so that if someone reports a bug we can quickly see their base
configuration. Having it completely configurable during runtime is a
bit of a pain in this regard though.. but most people will likely just
set cryptmode in modprobe.conf or something. Thoughts?
> +};
> +
> static inline const char *ath10k_cal_mode_str(enum ath10k_cal_mode mode)
> {
> switch (mode) {
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
> b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 89eb16b..a7df05d 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1018,8 +1018,7 @@ static void ath10k_htt_rx_h_undecap_raw(struct ath10k
> *ar,
>
> /* In most cases this will be true for sniffed frames. It makes sense
> * to deliver them as-is without stripping the crypto param. This
> would
> -* also make sense for software based decryption (which is not
> -* implemented in ath10k).
> +* also make sense for software based decryption.
I guess you should update the comment even more. The "would" doesn't
fit anymore. Instead: "This is necessary for software crypto too. "
Nonetheless kudos for taking care to update comments.
[...]
> 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_ENTRIES0
> +#define TARGET_10X_MAX_FRAG_ENTRIES10
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.
[...]
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
> b/drivers/net/wireless/ath/ath10k/wmi.c
> index 77220b0..1202150 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -508,7 +508,7 @@ static struct wmi_vdev_param_map wmi_10x_vdev_param_map =
> {
> .txbf = WMI_VDEV_PARAM_UNSUPPORTED,
> .packet_powersave = WMI_VDEV_PARAM_UNSUPPORTED,
> .drop_unencry = WMI_VDEV_PARAM_UNSUPPORTED,
> - .tx_encap_type = WMI_VDEV_PARAM_UNSUPPORTED,
> + .tx_encap_type = WMI_10X_VDEV_PARAM_TX_ENCAP_TYPE,
Hmm..
Technically this isn't correct because 10.1 doesn't support this vdev
parameter. Practically this
Re: [PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.
On Tue, Jun 2, 2015 at 12:17 AM, Michal Kazior wrote:
> On 1 June 2015 at 21:44, David Liu 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)
>
> [...]
>> @@ -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:
0Use HW crypto engine only. This uses native WiFi mode Tx/Rx encap
1Use SW crypto engine only. This uses raw mode Tx/Rx encap
2Supports 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.
Ben's CT 10.1 FW is a special use case and would fit into cryptmode=0
where his FW internally does the magic to disable HW Rx which is
independent to the driver.
Once Ben's FW is ready for raw mode, driver then can switch to use
cryptmode=1 or 2.
>
>> };
>>
>> enum ath10k_cal_mode {
>> @@ -492,6 +505,15 @@ enum ath10k_cal_mode {
>> ATH10K_CAL_MODE_DT,
>> };
>>
>> +enum ath10k_crypt_mode {
>> + /* Use HW crypto engine only */
>> + ATH10K_CRYPT_MODE_HW,
>> + /* HW SW crypto engine only (ie. HW crypto engine disabled) */
>> + ATH10K_CRYPT_MODE_SW,
>> + /* Both SW & HW crypto engine supported */
>> + ATH10K_CRYPT_MODE_HW_SW,
>
> I don't think this is clear enough (and the comments don't help at all):
>
> ATH10K_CRYPT_MODE_HW,
> ATH10K_CRYPT_MODE_SW,
> ATH10K_CRYPT_MODE_SW_RX_HW_TX,
>
> It would also be nice to have some sort of indication in driver
> logs/dmesg (without any debug_masks) as to what crypto mode driver
> uses so that if someone reports a bug we can quickly see their base
> configuration. Having it completely configurable during runtime is a
> bit of a pain in this regard though.. but most people will likely just
> set cryptmode in modprobe.conf or something. Thoughts?
I will add a log line to print the cryptmode param value.
>
>
>> +};
>> +
>> static inline const char *ath10k_cal_mode_str(enum ath10k_cal_mode mode)
>> {
>> switch (mode) {
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index 89eb16b..a7df05d 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -1018,8 +1018,7 @@ static void ath10k_htt_rx_h_undecap_raw(struct ath10k
>> *ar,
>>
>> /* In most cases this will be true for sniffed frames. It makes sense
>> * to deliver them as-is without stripping the crypto param. This
>> would
>> -* also make sense for software based decryption (which is not
>> -* implemented in ath10k).
>> +* also make sense for software based decryption.
>
> I g
Re: [PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.
On 2 June 2015 at 20:21, Liu CF/TW wrote:
> On Tue, Jun 2, 2015 at 12:17 AM, Michal Kazior
> wrote:
>> On 1 June 2015 at 21:44, David Liu 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:
>
>0Use HW crypto engine only. This uses native WiFi mode Tx/Rx encap
>
>1Use SW crypto engine only. This uses raw mode Tx/Rx encap
>
>2Supports 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_ENTRIES0
>>> +#define TARGET_10X_MAX_FRAG_ENTRIES10
>>
>> 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
Re: [PATCH 1/2] ath10k: add cryptmode param to support sw crypto and raw tx injection.
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_ENTRIES0
+#define TARGET_10X_MAX_FRAG_ENTRIES10
>>>
>>> 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.
>
Yeah you are right. I checked the QCA's PDF again and indeed it's for
Tx not for Rx.
David
>
> Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
