Re: [PATCH v3] ath10k: add 'cryptmode' param to support raw tx injection and software crypto

2015-06-11 Thread Kalle Valo
Liu CF/TW cfliu...@gmail.com writes:

 This change supports hardware crypto engine bypass by enabling raw
 Rx/Tx encap mode. This enables use cases such as software crypto and raw
 tx injection. This change introduces a new module param 'cryptmode'.

[...]

 --- a/drivers/net/wireless/ath/ath10k/debug.c
 +++ b/drivers/net/wireless/ath/ath10k/debug.c
 @@ -124,7 +124,7 @@ EXPORT_SYMBOL(ath10k_info);
  
  void ath10k_print_driver_info(struct ath10k *ar)
  {
 - ath10k_info(ar, %s (0x%08x, 0x%08x%s%s%s) fw %s api %d htt %d.%d wmi 
 %d cal %s max_sta %d\n,
 + ath10k_info(ar, %s (0x%08x, 0x%08x%s%s%s) fw %s api %d htt %d.%d wmi 
 %d cal %s max_sta %d flags 0x%lu\n,
   ar-hw_params.name,
   ar-target_version,
   ar-chip_id,
 @@ -138,7 +138,8 @@ void ath10k_print_driver_info(struct ath10k *ar)
   ar-htt.target_version_minor,
   ar-wmi.op_version,
   ath10k_cal_mode_str(ar-cal_mode),
 - ar-max_num_stations);
 + ar-max_num_stations,
 + ar-dev_flags);

What's the goal here? Printing a bitmap in hex is just gibberish, people
would always have to count the bits etc to get anything useful
information out from that print.

I would prefer the messages in more readable format. For example, if
idea is to show if cryptmode is enabled or not can't we just directly
print that like with format cryptmode %d?

-- 
Kalle Valo
--
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


Re: [PATCH v3] ath10k: add 'cryptmode' param to support raw tx injection and software crypto

2015-06-11 Thread Liu CF/TW
On Thu, Jun 11, 2015 at 6:59 AM, Kalle Valo kv...@qca.qualcomm.com wrote:
 Liu CF/TW cfliu...@gmail.com writes:

 This change supports hardware crypto engine bypass by enabling raw
 Rx/Tx encap mode. This enables use cases such as software crypto and raw
 tx injection. This change introduces a new module param 'cryptmode'.

 cryptmode:

   0: Use hardware crypto engine globally with native Wi-Fi mode TX/RX
  encapsulation to the firmware. This is the default mode.
   1: Use sofware crypto engine globally with raw mode TX/RX encapsulation
  to the firmware.

 These two values are ok.

   2: Supports both hardware and software crypto with raw mode TX/RX
  encapsulation to the firmware. By default hardware crypto engine is
  used. To use software crypto in this mode, set the per ath10k_vif
  'nohwcrypt' flag value to True.*
  *) The patch for setting vif specific 'nohwcrypt' flag when
 cryptmode=2 would be a separate patch to mac80211.

 But this the problematic one. I cannot apply something to ath10k until
 Johannes applies the mac80211 part. Didn't I mention this already
 earlier? At least I was supposed to do that.


Yes I already removed those mac80211 dependent changes in v3. So this
patch doesn't really depend on any mac80211 change.


 And most importantly does Johannes even agree with the approach? IIRC he
 was pretty reluctant about configuring the crypto mode via nl80211.

 I suggest splitting the patch into two: patch 1 adding support for
 cryptmode values 0 and 1, patch 2 adding support for cryptmode 2. That
 way we can commit patch 1 early and see what we can do with patch 2.

Thanks. Will do in v4.
Removing cryptmode=2 is just a enum change. No additional changes in
the business logic required.

After your review of v1, I have implemented knobs in both mac80211's
debugfs and nl80211. I will send an RFC to mac80211 ML to see which
one they prefer.


 --
 Kalle Valo
--
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


Re: [PATCH v3] ath10k: add 'cryptmode' param to support raw tx injection and software crypto

2015-06-11 Thread Liu CF/TW
On Thu, Jun 11, 2015 at 7:03 AM, Kalle Valo kv...@qca.qualcomm.com wrote:
 Liu CF/TW cfliu...@gmail.com writes:

 This change supports hardware crypto engine bypass by enabling raw
 Rx/Tx encap mode. This enables use cases such as software crypto and raw
 tx injection. This change introduces a new module param 'cryptmode'.

 [...]

 --- a/drivers/net/wireless/ath/ath10k/debug.c
 +++ b/drivers/net/wireless/ath/ath10k/debug.c
 @@ -124,7 +124,7 @@ EXPORT_SYMBOL(ath10k_info);

  void ath10k_print_driver_info(struct ath10k *ar)
  {
 - ath10k_info(ar, %s (0x%08x, 0x%08x%s%s%s) fw %s api %d htt %d.%d wmi 
 %d cal %s max_sta %d\n,
 + ath10k_info(ar, %s (0x%08x, 0x%08x%s%s%s) fw %s api %d htt %d.%d wmi 
 %d cal %s max_sta %d flags 0x%lu\n,
   ar-hw_params.name,
   ar-target_version,
   ar-chip_id,
 @@ -138,7 +138,8 @@ void ath10k_print_driver_info(struct ath10k *ar)
   ar-htt.target_version_minor,
   ar-wmi.op_version,
   ath10k_cal_mode_str(ar-cal_mode),
 - ar-max_num_stations);
 + ar-max_num_stations,
 + ar-dev_flags);

 What's the goal here? Printing a bitmap in hex is just gibberish, people
 would always have to count the bits etc to get anything useful
 information out from that print.

 I would prefer the messages in more readable format. For example, if
 idea is to show if cryptmode is enabled or not can't we just directly
 print that like with format cryptmode %d?

Sure. will do in v4.


 --
 Kalle Valo
--
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


Re: [PATCH v3] ath10k: add 'cryptmode' param to support raw tx injection and software crypto

2015-06-11 Thread Liu CF/TW
Thanks Michal for the tests you did and the additional patch to
address AMSDU limitation.
Per our discussions offline, in v4, I will squash your patch and add
your test results in the commit log.
I will also put a note that AMSDU be a known limitation when cryptmode!=0.

Thanks
David

On Tue, Jun 9, 2015 at 5:40 AM, Michal Kazior michal.kaz...@tieto.com wrote:
 On 9 June 2015 at 03:13, Liu CF/TW cfliu...@gmail.com wrote:
 This change supports hardware crypto engine bypass by enabling raw
 Rx/Tx encap mode. This enables use cases such as software crypto and raw
 tx injection. This change introduces a new module param 'cryptmode'.

 cryptmode:

   0: Use hardware crypto engine globally with native Wi-Fi mode TX/RX
  encapsulation to the firmware. This is the default mode.
   1: Use sofware crypto engine globally with raw mode TX/RX encapsulation
  to the firmware.
   2: Supports both hardware and software crypto with raw mode TX/RX
  encapsulation to the firmware. By default hardware crypto engine is
  used. To use software crypto in this mode, set the per ath10k_vif
  'nohwcrypt' flag value to True.*
  *) The patch for setting vif specific 'nohwcrypt' flag when
 cryptmode=2 would be a separate patch to mac80211.

 Possible use case examples:

   - Use software crypto engine in mac80211. (cryptmode=1)

   - Support inject raw unencrypted frame on monitor interface and use
 hardware crypto to encrypt the injected Tx frames. (cryptmode=2)

   - Support receive raw hardware decrypted frame with encryption header
 on monitor interface. (cryptmode=2)

   - Support hybrid local  split MAC mode to support tunneling protocols
 such as CAPWAP: Use hardware crypto for BSS in local mode,
 and bypass hardware crypto for BSS in split MAC mode.
 (cryptmode=2, ath10k_vif nohwcrypt=0 for local mode, =1 for split MAC
  mode)

 Testing:

   Used QCA988x hw 2.0 with firmware-4.bin_10.2.4.48 with
   backports-20150424.

   All test case tested** with hostapd in both WPA2-PSK-TKIP (11g) and
   WPA2-PSK-CCMP(11n/ac). Verified ping and http to google.com works.

   **) Need to skip ATH10K_FW_FEATURE_RAW_MODE_SUPPORT check in core.c to
   test firmware. After all, none of the existing QCA official firmware
   exports that firmware bit yet.

   Test Casecryptmode value tested
   ---
   1. ath10k hardware crypto can encrypt/decrypt   0: PASS
  data frames when hostapd config the BSS in   1: Not applicable.
  WPA2-PSK-TKIP and WPA2-PSK-CCMP modes.   2: PASS

   2. mac80211 software crypto can encrypt/decrypt 0: Not applicable
  data frames when hostapd config the BSS in   1: PASS
  WPA2-PSK-TKIP and WPA2-PSK-CCMP modes.   2: PASS, when vif
  nohwcrypt=1

   3. Monitor interface Tx: User application can   0: Not applicable
  inject unencrypted raw Tx frames to monitor  1: PASS (mac80211)
  interface for mac80211 or hardware to encrypt2: PASS (hardware)
  the frames.

   4. Monitor interface Rx: mac80211 software crypto   0: Not applicable
  engine can decrypt received TKIP/CCMP frames.1: PASS
  User application see decrypted frames.   2: PASS, when vif
  nohwcrypt=1

   5. CAPWAP-like local and split MAC datapath 0: Not applicable
  tunneling: Setup BSS1=Local MAC mode on wlan0,   1: Not applicable
  BSS2=Split MAC mode on wlan0_monitor interface.  2: PASS
  Test BSS1 data frames can be encrypted and
  decrypted by ath10k hardware crypto engine
  while BSS2 data frames can skip both hardware 
  kernel mac80211 crypto engines via monitor
  interface to the user application fot tunneling.

 I've finally got to run a few tests on your patch.

 I've tried qca988x with 10.2.4.48-2 (fw_feature hardcoded in driver).

 Here's a short summary of my findings:

  - cryptmode=1 can fix 4addr+802.1q VLAN tagging in both AP and STA
 modes (a problem recently reported by Cedric),
  - your patch as-is has AP WEP broken, see below
  - your patch as-is has TCP broken, see below
  - your patch as-is has performance really broken, see below
  - once patched it looks pretty solid.


 At the end of this email I'm attaching (most likely whitespace
 damaged) patch I've made on top of your patch for reviewing
 convenience.

 For testing/merging convenience here's a paste link: http://paste.ee/p/YMFrO


 Performance:

 cryptmode=1
  ap=qca988x sta=killer1525
   killer1525  -  qca988x 194.496 mbps [tcp1 ip4]
   killer1525  -  qca988x 238.309 mbps [tcp5 ip4]
   killer1525  -  qca988x 266.958 mbps [udp1 ip4]
   killer1525  -  qca988x 477.468 mbps [udp5 ip4]
   qca988x -  killer1525  301.378 mbps [tcp1 ip4]
   qca988x -  killer1525  297.949 

Re: [PATCH v3] ath10k: add 'cryptmode' param to support raw tx injection and software crypto

2015-06-11 Thread Kalle Valo
Liu CF/TW cfliu...@gmail.com writes:

 This change supports hardware crypto engine bypass by enabling raw
 Rx/Tx encap mode. This enables use cases such as software crypto and raw
 tx injection. This change introduces a new module param 'cryptmode'.

 cryptmode:

   0: Use hardware crypto engine globally with native Wi-Fi mode TX/RX
  encapsulation to the firmware. This is the default mode.
   1: Use sofware crypto engine globally with raw mode TX/RX encapsulation
  to the firmware.

These two values are ok.

   2: Supports both hardware and software crypto with raw mode TX/RX
  encapsulation to the firmware. By default hardware crypto engine is
  used. To use software crypto in this mode, set the per ath10k_vif
  'nohwcrypt' flag value to True.*
  *) The patch for setting vif specific 'nohwcrypt' flag when
 cryptmode=2 would be a separate patch to mac80211.

But this the problematic one. I cannot apply something to ath10k until
Johannes applies the mac80211 part. Didn't I mention this already
earlier? At least I was supposed to do that.

And most importantly does Johannes even agree with the approach? IIRC he
was pretty reluctant about configuring the crypto mode via nl80211.

I suggest splitting the patch into two: patch 1 adding support for
cryptmode values 0 and 1, patch 2 adding support for cryptmode 2. That
way we can commit patch 1 early and see what we can do with patch 2.

-- 
Kalle Valo
--
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


Re: [PATCH v3] ath10k: add 'cryptmode' param to support raw tx injection and software crypto

2015-06-09 Thread Michal Kazior
On 9 June 2015 at 03:13, Liu CF/TW cfliu...@gmail.com wrote:
 This change supports hardware crypto engine bypass by enabling raw
 Rx/Tx encap mode. This enables use cases such as software crypto and raw
 tx injection. This change introduces a new module param 'cryptmode'.

 cryptmode:

   0: Use hardware crypto engine globally with native Wi-Fi mode TX/RX
  encapsulation to the firmware. This is the default mode.
   1: Use sofware crypto engine globally with raw mode TX/RX encapsulation
  to the firmware.
   2: Supports both hardware and software crypto with raw mode TX/RX
  encapsulation to the firmware. By default hardware crypto engine is
  used. To use software crypto in this mode, set the per ath10k_vif
  'nohwcrypt' flag value to True.*
  *) The patch for setting vif specific 'nohwcrypt' flag when
 cryptmode=2 would be a separate patch to mac80211.

 Possible use case examples:

   - Use software crypto engine in mac80211. (cryptmode=1)

   - Support inject raw unencrypted frame on monitor interface and use
 hardware crypto to encrypt the injected Tx frames. (cryptmode=2)

   - Support receive raw hardware decrypted frame with encryption header
 on monitor interface. (cryptmode=2)

   - Support hybrid local  split MAC mode to support tunneling protocols
 such as CAPWAP: Use hardware crypto for BSS in local mode,
 and bypass hardware crypto for BSS in split MAC mode.
 (cryptmode=2, ath10k_vif nohwcrypt=0 for local mode, =1 for split MAC
  mode)

 Testing:

   Used QCA988x hw 2.0 with firmware-4.bin_10.2.4.48 with
   backports-20150424.

   All test case tested** with hostapd in both WPA2-PSK-TKIP (11g) and
   WPA2-PSK-CCMP(11n/ac). Verified ping and http to google.com works.

   **) Need to skip ATH10K_FW_FEATURE_RAW_MODE_SUPPORT check in core.c to
   test firmware. After all, none of the existing QCA official firmware
   exports that firmware bit yet.

   Test Casecryptmode value tested
   ---
   1. ath10k hardware crypto can encrypt/decrypt   0: PASS
  data frames when hostapd config the BSS in   1: Not applicable.
  WPA2-PSK-TKIP and WPA2-PSK-CCMP modes.   2: PASS

   2. mac80211 software crypto can encrypt/decrypt 0: Not applicable
  data frames when hostapd config the BSS in   1: PASS
  WPA2-PSK-TKIP and WPA2-PSK-CCMP modes.   2: PASS, when vif
  nohwcrypt=1

   3. Monitor interface Tx: User application can   0: Not applicable
  inject unencrypted raw Tx frames to monitor  1: PASS (mac80211)
  interface for mac80211 or hardware to encrypt2: PASS (hardware)
  the frames.

   4. Monitor interface Rx: mac80211 software crypto   0: Not applicable
  engine can decrypt received TKIP/CCMP frames.1: PASS
  User application see decrypted frames.   2: PASS, when vif
  nohwcrypt=1

   5. CAPWAP-like local and split MAC datapath 0: Not applicable
  tunneling: Setup BSS1=Local MAC mode on wlan0,   1: Not applicable
  BSS2=Split MAC mode on wlan0_monitor interface.  2: PASS
  Test BSS1 data frames can be encrypted and
  decrypted by ath10k hardware crypto engine
  while BSS2 data frames can skip both hardware 
  kernel mac80211 crypto engines via monitor
  interface to the user application fot tunneling.

I've finally got to run a few tests on your patch.

I've tried qca988x with 10.2.4.48-2 (fw_feature hardcoded in driver).

Here's a short summary of my findings:

 - cryptmode=1 can fix 4addr+802.1q VLAN tagging in both AP and STA
modes (a problem recently reported by Cedric),
 - your patch as-is has AP WEP broken, see below
 - your patch as-is has TCP broken, see below
 - your patch as-is has performance really broken, see below
 - once patched it looks pretty solid.


At the end of this email I'm attaching (most likely whitespace
damaged) patch I've made on top of your patch for reviewing
convenience.

For testing/merging convenience here's a paste link: http://paste.ee/p/YMFrO


Performance:

cryptmode=1
 ap=qca988x sta=killer1525
  killer1525  -  qca988x 194.496 mbps [tcp1 ip4]
  killer1525  -  qca988x 238.309 mbps [tcp5 ip4]
  killer1525  -  qca988x 266.958 mbps [udp1 ip4]
  killer1525  -  qca988x 477.468 mbps [udp5 ip4]
  qca988x -  killer1525  301.378 mbps [tcp1 ip4]
  qca988x -  killer1525  297.949 mbps [tcp5 ip4]
  qca988x -  killer1525  331.351 mbps [udp1 ip4]
  qca988x -  killer1525  371.528 mbps [udp5 ip4]
 ap=killer1525 sta=qca988x
  qca988x -  killer1525  331.447 mbps [tcp1 ip4]
  qca988x -  killer1525  328.783 mbps [tcp5 ip4]
  qca988x -  killer1525  375.309 mbps [udp1 ip4]
  qca988x -  killer1525  403.379 mbps [udp5 ip4]
  killer1525  -  qca988x 

[PATCH v3] ath10k: add 'cryptmode' param to support raw tx injection and software crypto

2015-06-08 Thread Liu CF/TW
This change supports hardware crypto engine bypass by enabling raw
Rx/Tx encap mode. This enables use cases such as software crypto and raw
tx injection. This change introduces a new module param 'cryptmode'.

cryptmode:

  0: Use hardware crypto engine globally with native Wi-Fi mode TX/RX
 encapsulation to the firmware. This is the default mode.
  1: Use sofware crypto engine globally with raw mode TX/RX encapsulation
 to the firmware.
  2: Supports both hardware and software crypto with raw mode TX/RX
 encapsulation to the firmware. By default hardware crypto engine is
 used. To use software crypto in this mode, set the per ath10k_vif
 'nohwcrypt' flag value to True.*
 *) The patch for setting vif specific 'nohwcrypt' flag when
cryptmode=2 would be a separate patch to mac80211.

Possible use case examples:

  - Use software crypto engine in mac80211. (cryptmode=1)

  - Support inject raw unencrypted frame on monitor interface and use
hardware crypto to encrypt the injected Tx frames. (cryptmode=2)

  - Support receive raw hardware decrypted frame with encryption header
on monitor interface. (cryptmode=2)

  - Support hybrid local  split MAC mode to support tunneling protocols
such as CAPWAP: Use hardware crypto for BSS in local mode,
and bypass hardware crypto for BSS in split MAC mode.
(cryptmode=2, ath10k_vif nohwcrypt=0 for local mode, =1 for split MAC
 mode)

Testing:

  Used QCA988x hw 2.0 with firmware-4.bin_10.2.4.48 with
  backports-20150424.

  All test case tested** with hostapd in both WPA2-PSK-TKIP (11g) and
  WPA2-PSK-CCMP(11n/ac). Verified ping and http to google.com works.

  **) Need to skip ATH10K_FW_FEATURE_RAW_MODE_SUPPORT check in core.c to
  test firmware. After all, none of the existing QCA official firmware
  exports that firmware bit yet.

  Test Casecryptmode value tested
  ---
  1. ath10k hardware crypto can encrypt/decrypt   0: PASS
 data frames when hostapd config the BSS in   1: Not applicable.
 WPA2-PSK-TKIP and WPA2-PSK-CCMP modes.   2: PASS

  2. mac80211 software crypto can encrypt/decrypt 0: Not applicable
 data frames when hostapd config the BSS in   1: PASS
 WPA2-PSK-TKIP and WPA2-PSK-CCMP modes.   2: PASS, when vif
 nohwcrypt=1

  3. Monitor interface Tx: User application can   0: Not applicable
 inject unencrypted raw Tx frames to monitor  1: PASS (mac80211)
 interface for mac80211 or hardware to encrypt2: PASS (hardware)
 the frames.

  4. Monitor interface Rx: mac80211 software crypto   0: Not applicable
 engine can decrypt received TKIP/CCMP frames.1: PASS
 User application see decrypted frames.   2: PASS, when vif
 nohwcrypt=1

  5. CAPWAP-like local and split MAC datapath 0: Not applicable
 tunneling: Setup BSS1=Local MAC mode on wlan0,   1: Not applicable
 BSS2=Split MAC mode on wlan0_monitor interface.  2: PASS
 Test BSS1 data frames can be encrypted and 
 decrypted by ath10k hardware crypto engine 
 while BSS2 data frames can skip both hardware 
 kernel mac80211 crypto engines via monitor
 interface to the user application fot tunneling.

Signed-off-by: Liu CF/TW cfliu...@gmail.com
---
 drivers/net/wireless/ath/ath10k/core.c| 31 ++
 drivers/net/wireless/ath/ath10k/core.h| 33 +--
 drivers/net/wireless/ath/ath10k/debug.c   |  5 +--
 drivers/net/wireless/ath/ath10k/htt_rx.c  |  5 ++-
 drivers/net/wireless/ath/ath10k/htt_tx.c  |  9 +-
 drivers/net/wireless/ath/ath10k/hw.h  | 11 +++
 drivers/net/wireless/ath/ath10k/mac.c | 54 ++-
 drivers/net/wireless/ath/ath10k/wmi-tlv.c |  2 +-
 drivers/net/wireless/ath/ath10k/wmi.c |  8 ++---
 9 files changed, 129 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 59496a9..ea1c43f 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;
+static unsigned int 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, uint, 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-hardware, 1-software, 2-both);
 
 static const struct ath10k_hw_params ath10k_hw_params_list[] = {