All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] remove custom Michael MIC implementation
@ 2017-03-31  4:47 Tobin C. Harding
  2017-03-31  4:47 ` [PATCH RFC] staging: ks7010: " Tobin C. Harding
  2017-03-31  7:58 ` [PATCH RFC] " Wolfram Sang
  0 siblings, 2 replies; 19+ messages in thread
From: Tobin C. Harding @ 2017-03-31  4:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tobin C. Harding, Greg Kroah-Hartman, Wolfram Sang, devel,
	Tycho Andersen

This RFC can be applied on op on Linus' tree 89970a0

Configuration options needed to build are

CONFIG_STAGING=y
CONFIG_KS7010=m

The ks7010 driver currently uses a custom implementation of the
Michael MIC algorithm. The kernel has an implementation of this
algorithm already. This patch is an attempt to replace the custom
implementation with the in-tree implementation via the kernel
cryptography API.

I am not an expert on the Michael Message Integrity Check or on
cryptography in general. Actually, I'm not even an expert on kernel
development.

I believe I have mirrored the behavior of the custom implementation. I
do not know if I have done this completely correctly or, for that
matter, if I have gone about it correctly. The only in-tree driver I
could find that does the MIC check in software was the Orinoco driver
(net/wireless/intersil/orinoco). I based this code off of the Orinoco
code and the current implementation.

The whole thing is in one patch since there was no way to keep the
driver in a sane state during the implementation replacement.

The steps I took were as follows;

1. Remove the custom implementation, michael_mic.[ch]

2. Implement helper functions that call the kernel crypto API, this is
   the code that is based of Orinoco, mic.[ch]

3. Replace driver calls to the custom implementation with calls to the
   newly defined helper functions.


The code is untested, I have hardware in the mail.

If any one is interested and has any comments I would really like to
hear them. I am open to all suggestions (even down to trivial coding
style issues).

Thank you for taking the time to read this and for any tips you may be
able to give me.

thanks,
Tobin.

Tobin C. Harding (1):
  staging: ks7010: remove custom Michael MIC implementation

 drivers/staging/ks7010/Makefile      |   2 +-
 drivers/staging/ks7010/ks_hostif.c   |  48 +++++++-----
 drivers/staging/ks7010/ks_wlan.h     |   3 +
 drivers/staging/ks7010/mic.c         | 131 +++++++++++++++++++++++++++++++
 drivers/staging/ks7010/mic.h         |  22 ++++++
 drivers/staging/ks7010/michael_mic.c | 148 -----------------------------------
 drivers/staging/ks7010/michael_mic.h |  25 ------
 7 files changed, 186 insertions(+), 193 deletions(-)
 create mode 100644 drivers/staging/ks7010/mic.c
 create mode 100644 drivers/staging/ks7010/mic.h
 delete mode 100644 drivers/staging/ks7010/michael_mic.c
 delete mode 100644 drivers/staging/ks7010/michael_mic.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH RFC] staging: ks7010: remove custom Michael MIC implementation
  2017-03-31  4:47 [PATCH RFC] remove custom Michael MIC implementation Tobin C. Harding
@ 2017-03-31  4:47 ` Tobin C. Harding
  2017-03-31  5:16   ` Joe Perches
  2017-03-31  7:58 ` [PATCH RFC] " Wolfram Sang
  1 sibling, 1 reply; 19+ messages in thread
From: Tobin C. Harding @ 2017-03-31  4:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tobin C. Harding, Greg Kroah-Hartman, Wolfram Sang, devel,
	Tycho Andersen

ks7010 currently uses a custom implementation of the Michael MIC
algorithm. The kernel has an implementation of this algorithm
already, we should use it.

Remove the custom implementation. Implement helper functions that call
the in-tree implementation through the crypto API. Update the
makefile. Replace driver calls to the custom implementation with calls
to the newly defined helper functions.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/Makefile      |   2 +-
 drivers/staging/ks7010/ks_hostif.c   |  48 +++++++-----
 drivers/staging/ks7010/ks_wlan.h     |   3 +
 drivers/staging/ks7010/mic.c         | 131 +++++++++++++++++++++++++++++++
 drivers/staging/ks7010/mic.h         |  22 ++++++
 drivers/staging/ks7010/michael_mic.c | 148 -----------------------------------
 drivers/staging/ks7010/michael_mic.h |  25 ------
 7 files changed, 186 insertions(+), 193 deletions(-)
 create mode 100644 drivers/staging/ks7010/mic.c
 create mode 100644 drivers/staging/ks7010/mic.h
 delete mode 100644 drivers/staging/ks7010/michael_mic.c
 delete mode 100644 drivers/staging/ks7010/michael_mic.h

diff --git a/drivers/staging/ks7010/Makefile b/drivers/staging/ks7010/Makefile
index 69fcf8d..195b570 100644
--- a/drivers/staging/ks7010/Makefile
+++ b/drivers/staging/ks7010/Makefile
@@ -1,4 +1,4 @@
 obj-$(CONFIG_KS7010) += ks7010.o
 
 ccflags-y 	     += -DKS_WLAN_DEBUG=0
-ks7010-y	     := michael_mic.o ks_hostif.o ks_wlan_net.o ks7010_sdio.o
+ks7010-y	     := mic.o ks_hostif.o ks_wlan_net.o ks7010_sdio.o
diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
index da7c42e..68ecbf7 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -12,7 +12,7 @@
 #include "ks_wlan.h"
 #include "ks_hostif.h"
 #include "eap_packet.h"
-#include "michael_mic.h"
+#include "mic.h"
 
 #include <linux/etherdevice.h>
 #include <linux/if_ether.h>
@@ -315,7 +315,6 @@ void hostif_data_indication(struct ks_wlan_private *priv)
 	unsigned short auth_type;
 	unsigned char temp[256];
 
-	unsigned char RecvMIC[8];
 	char buf[128];
 	struct ether_hdr *eth_hdr;
 	unsigned short eth_proto;
@@ -323,8 +322,8 @@ void hostif_data_indication(struct ks_wlan_private *priv)
 	struct mic_failure_t *mic_failure;
 	struct ieee802_1x_hdr *aa1x_hdr;
 	struct wpa_eapol_key *eap_key;
-	struct michel_mic_t michel_mic;
 	union iwreq_data wrqu;
+	struct wpa_key_t *key;
 
 	DPRINTK(3, "\n");
 
@@ -337,6 +336,7 @@ void hostif_data_indication(struct ks_wlan_private *priv)
 
 	auth_type = get_WORD(priv);	/* AuthType */
 	get_WORD(priv);	/* Reserve Area */
+	key = &priv->wpa.key[auth_type - 1];
 
 	eth_hdr = (struct ether_hdr *)(priv->rxp);
 	eth_proto = ntohs(eth_hdr->h_proto);
@@ -372,18 +372,25 @@ void hostif_data_indication(struct ks_wlan_private *priv)
 				 && priv->wpa.group_suite ==
 				 IW_AUTH_CIPHER_TKIP))
 			    && priv->wpa.key[auth_type - 1].key_len) {
+				u8 micrx[MICHAEL_MIC_LEN];
+				u8 mic[MICHAEL_MIC_LEN];
+				u8 *addr;
 				DPRINTK(4, "TKIP: protocol=%04X: size=%u\n",
 					eth_proto, priv->rx_size);
-				/* MIC save */
-				memcpy(&RecvMIC[0],
-				       (priv->rxp) + ((priv->rx_size) - 8), 8);
-				priv->rx_size = priv->rx_size - 8;
+
+				addr = priv->rxp + priv->rx_size - MICHAEL_MIC_LEN;
+				memcpy(micrx, addr, MICHAEL_MIC_LEN);
+				priv->rx_size -= MICHAEL_MIC_LEN;
+
 				if (auth_type > 0 && auth_type < 4) {	/* auth_type check */
-					MichaelMICFunction(&michel_mic, (uint8_t *) priv->wpa.key[auth_type - 1].rx_mic_key, (uint8_t *) priv->rxp, (int)priv->rx_size, (uint8_t) 0,	/* priority */
-							   (uint8_t *)
-							   michel_mic.Result);
+					u8 priority = 0;
+					ks_wlan_mic(priv->rx_tfm_mic,
+						    key->rx_mic_key, priority,
+						    priv->rxp, priv->rx_size,
+						    mic);
 				}
-				if (memcmp(michel_mic.Result, RecvMIC, 8)) {
+
+				if (memcmp(mic, micrx, MICHAEL_MIC_LEN) != 0) {
 					now = jiffies;
 					mic_failure = &priv->wpa.mic_failure;
 					/* MIC FAILURE */
@@ -1112,7 +1119,6 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet)
 	int result = 0;
 	unsigned short eth_proto;
 	struct ether_hdr *eth_hdr;
-	struct michel_mic_t michel_mic;
 	unsigned short keyinfo = 0;
 	struct ieee802_1x_hdr *aa1x_hdr;
 	struct wpa_eapol_key *eap_key;
@@ -1220,13 +1226,17 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *packet)
 			pp->auth_type = cpu_to_le16((uint16_t) TYPE_AUTH);	/* no encryption */
 		} else {
 			if (priv->wpa.pairwise_suite == IW_AUTH_CIPHER_TKIP) {
-				MichaelMICFunction(&michel_mic, (uint8_t *) priv->wpa.key[0].tx_mic_key, (uint8_t *) &pp->data[0], (int)packet_len, (uint8_t) 0,	/* priority */
-						   (uint8_t *) michel_mic.
-						   Result);
-				memcpy(p, michel_mic.Result, 8);
-				length += 8;
-				packet_len += 8;
-				p += 8;
+				struct wpa_key_t *key = &priv->wpa.key[0];
+				u8 priority = 0;
+				u8 mic[MICHAEL_MIC_LEN];
+
+				ks_wlan_mic(priv->tx_tfm_mic, key->tx_mic_key,
+					    priority, pp->data, packet_len, mic);
+
+				memcpy(p, mic, MICHAEL_MIC_LEN);
+				length += MICHAEL_MIC_LEN;
+				packet_len += MICHAEL_MIC_LEN;
+				p += MICHAEL_MIC_LEN;
 				pp->auth_type =
 				    cpu_to_le16((uint16_t) TYPE_DATA);
 
diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index 9ab80e1..be475fa 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -496,6 +496,9 @@ struct ks_wlan_private {
 	unsigned long last_wakeup;
 
 	uint wakeup_count;	/* for detect wakeup loop */
+
+	struct crypto_shash *rx_tfm_mic;
+	struct crypto_shash *tx_tfm_mic;
 };
 
 int ks_wlan_net_start(struct net_device *dev);
diff --git a/drivers/staging/ks7010/mic.c b/drivers/staging/ks7010/mic.c
new file mode 100644
index 0000000..5c66fc6
--- /dev/null
+++ b/drivers/staging/ks7010/mic.c
@@ -0,0 +1,131 @@
+/*
+ *   MIC helpers
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License version 2 as
+ *   published by the Free Software Foundation.
+ *
+ *   Borrows heavily from drivers/net/wireless/intersil/orinoco
+ */
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/if_ether.h>
+#include <linux/scatterlist.h>
+#include <crypto/hash.h>
+
+#include "ks_wlan.h"
+#include "mic.h"
+
+/**
+ * ks_wlan_mic_init() - Michael MIC crypto setup.
+ *
+ * Calls kernel to locate Michael MIC algorithm and allocates
+ * receive and transmit transforms.
+ */
+int ks_wlan_mic_init(struct ks_wlan_private *priv)
+{
+	priv->tx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
+					      CRYPTO_ALG_ASYNC);
+	if (IS_ERR(priv->tx_tfm_mic)) {
+		pr_debug("ks_wlan_mic_init: could not allocate "
+		         "crypto API michael_mic\n");
+		priv->tx_tfm_mic = NULL;
+		return -ENOMEM;
+	}
+
+	priv->rx_tfm_mic = crypto_alloc_shash("michael_mic", 0,
+					      CRYPTO_ALG_ASYNC);
+	if (IS_ERR(priv->rx_tfm_mic)) {
+		pr_debug("ks_wlan_mic_init: could not allocate "
+		         "crypto API michael_mic\n");
+		priv->rx_tfm_mic = NULL;
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+/**
+ * ks_wlan_mic_free() - Free crypto data structures.
+ */
+void ks_wlan_mic_free(struct ks_wlan_private *priv)
+{
+	if (priv->tx_tfm_mic)
+		crypto_free_shash(priv->tx_tfm_mic);
+	if (priv->rx_tfm_mic)
+		crypto_free_shash(priv->rx_tfm_mic);
+}
+
+/**
+ * ks_wlan_mic() - Calculate Message Integrity Check
+ *
+ * @tfm_michael: Transform allocated prior.
+ * @key: Key value.
+ * @priority: Usually 0.
+ * @data: DA, SA, and data to check.
+ * @data_len: Length of data.
+ * @mic: MIC result (8 bytes).
+ *
+ * The destination (DA) and source (SA) addresses are passed in at the front of
+ * data. The message integrity check format is built up as specified
+ * by IEEE802.11. The MIC is calculated on @data *not* including DA and SA.
+ */
+int ks_wlan_mic(struct crypto_shash *tfm_michael, u8 *key,
+		u8 priority, u8 *data, size_t data_len, u8 *mic)
+{
+	SHASH_DESC_ON_STACK(desc, tfm_michael);
+	u8 hdr[ETH_HLEN + 2]; /* 16 bytes */
+	u8 *da, *sa;
+	int ret;
+
+	/*
+	 * IEEE802.11i  page 47
+	 * Figure 43g TKIP MIC processing format
+	 * +--+--+--------+--+----+--+--+--+--+--+--+--+--+
+	 * |6 |6 |1       |3 |M   |1 |1 |1 |1 |1 |1 |1 |1 | Octet
+	 * +--+--+--------+--+----+--+--+--+--+--+--+--+--+
+	 * |DA|SA|Priority|0 |Data|M0|M1|M2|M3|M4|M5|M6|M7|
+	 * +--+--+--------+--+----+--+--+--+--+--+--+--+--+
+	 */
+
+	if (!tfm_michael) {
+		pr_warning("ks_wlan_mic: tfm_michael == NULL\n");
+		return -EINVAL;
+	}
+
+	da = data;
+	sa = data + ETH_ALEN;
+	data += 2 * ETH_ALEN;
+	data_len -= 2 * ETH_ALEN;
+
+	memcpy(&hdr[0], da, ETH_ALEN);
+	memcpy(&hdr[ETH_ALEN], sa, ETH_ALEN);
+	hdr[ETH_ALEN * 2] = priority;
+	hdr[ETH_ALEN * 2 + 1] = 0;
+	hdr[ETH_ALEN * 2 + 2] = 0;
+	hdr[ETH_ALEN * 2 + 3] = 0;
+
+	desc->tfm = tfm_michael;
+	desc->flags = 0;
+
+	ret = crypto_shash_setkey(tfm_michael, key, MIC_KEY_SIZE);
+	if (ret)
+		return ret;
+
+	ret = crypto_shash_init(desc);
+	if (ret)
+		return ret;
+
+	ret = crypto_shash_update(desc, hdr, sizeof(hdr));
+	if (ret)
+		return ret;
+
+	ret = crypto_shash_update(desc, data, data_len);
+	if (ret)
+		return ret;
+
+	ret = crypto_shash_final(desc, mic);
+	shash_desc_zero(desc);
+
+	return ret;
+}
diff --git a/drivers/staging/ks7010/mic.h b/drivers/staging/ks7010/mic.h
new file mode 100644
index 0000000..eb048dc
--- /dev/null
+++ b/drivers/staging/ks7010/mic.h
@@ -0,0 +1,22 @@
+/*
+ *   MIC helpers
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License version 2 as
+ *   published by the Free Software Foundation.
+ */
+#ifndef _KS7010_MIC_H_
+#define _KS7010_MIC_H_
+
+#include <linux/types.h>
+#include <crypto/hash.h>
+#include "ks_wlan.h"
+
+#define MICHAEL_MIC_LEN 8
+
+int ks_wlan_mic_init(struct ks_wlan_private *priv);
+void ks_wlan_mic_free(struct ks_wlan_private *priv);
+int ks_wlan_mic(struct crypto_shash *tfm_michael, u8 *key,
+		u8 priority, u8 *data, size_t data_len, u8 *mic);
+
+#endif /* _KS7010_MIC_H_ */
diff --git a/drivers/staging/ks7010/michael_mic.c b/drivers/staging/ks7010/michael_mic.c
deleted file mode 100644
index f6e70fa..0000000
--- a/drivers/staging/ks7010/michael_mic.c
+++ /dev/null
@@ -1,148 +0,0 @@
-/*
- *   Driver for KeyStream wireless LAN
- *
- *   Copyright (C) 2005-2008 KeyStream Corp.
- *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
- */
-
-#include <linux/types.h>
-#include <linux/string.h>
-#include "michael_mic.h"
-
-// Rotation functions on 32 bit values
-#define ROL32(A, n)	(((A) << (n)) | (((A) >> (32 - (n))) & ((1UL << (n)) - 1)))
-#define ROR32(A, n)	ROL32((A), 32 - (n))
-// Convert from Byte[] to UInt32 in a portable way
-#define getUInt32(A, B)	((uint32_t)(A[B + 0] << 0) \
-		+ (A[B + 1] << 8) + (A[B + 2] << 16) + (A[B + 3] << 24))
-
-// Convert from UInt32 to Byte[] in a portable way
-#define putUInt32(A, B, C)					\
-do {								\
-	A[B + 0] = (uint8_t)(C & 0xff);				\
-	A[B + 1] = (uint8_t)((C >> 8) & 0xff);			\
-	A[B + 2] = (uint8_t)((C >> 16) & 0xff);			\
-	A[B + 3] = (uint8_t)((C >> 24) & 0xff);			\
-} while (0)
-
-// Reset the state to the empty message.
-#define MichaelClear(A)			\
-do {					\
-	A->L = A->K0;			\
-	A->R = A->K1;			\
-	A->nBytesInM = 0;		\
-} while (0)
-
-static
-void MichaelInitializeFunction(struct michel_mic_t *Mic, uint8_t *key)
-{
-	// Set the key
-	Mic->K0 = getUInt32(key, 0);
-	Mic->K1 = getUInt32(key, 4);
-
-	//clear();
-	MichaelClear(Mic);
-}
-
-#define MichaelBlockFunction(L, R)				\
-do {								\
-	R ^= ROL32(L, 17);					\
-	L += R;							\
-	R ^= ((L & 0xff00ff00) >> 8) | ((L & 0x00ff00ff) << 8);	\
-	L += R;							\
-	R ^= ROL32(L, 3);					\
-	L += R;							\
-	R ^= ROR32(L, 2);					\
-	L += R;							\
-} while (0)
-
-static
-void MichaelAppend(struct michel_mic_t *Mic, uint8_t *src, int nBytes)
-{
-	int addlen;
-
-	if (Mic->nBytesInM) {
-		addlen = 4 - Mic->nBytesInM;
-		if (addlen > nBytes)
-			addlen = nBytes;
-		memcpy(&Mic->M[Mic->nBytesInM], src, addlen);
-		Mic->nBytesInM += addlen;
-		src += addlen;
-		nBytes -= addlen;
-
-		if (Mic->nBytesInM < 4)
-			return;
-
-		Mic->L ^= getUInt32(Mic->M, 0);
-		MichaelBlockFunction(Mic->L, Mic->R);
-		Mic->nBytesInM = 0;
-	}
-
-	while (nBytes >= 4) {
-		Mic->L ^= getUInt32(src, 0);
-		MichaelBlockFunction(Mic->L, Mic->R);
-		src += 4;
-		nBytes -= 4;
-	}
-
-	if (nBytes > 0) {
-		Mic->nBytesInM = nBytes;
-		memcpy(Mic->M, src, nBytes);
-	}
-}
-
-static
-void MichaelGetMIC(struct michel_mic_t *Mic, uint8_t *dst)
-{
-	u8 *data = Mic->M;
-
-	switch (Mic->nBytesInM) {
-	case 0:
-		Mic->L ^= 0x5a;
-		break;
-	case 1:
-		Mic->L ^= data[0] | 0x5a00;
-		break;
-	case 2:
-		Mic->L ^= data[0] | (data[1] << 8) | 0x5a0000;
-		break;
-	case 3:
-		Mic->L ^= data[0] | (data[1] << 8) | (data[2] << 16) |
-		    0x5a000000;
-		break;
-	}
-	MichaelBlockFunction(Mic->L, Mic->R);
-	MichaelBlockFunction(Mic->L, Mic->R);
-	// The appendByte function has already computed the result.
-	putUInt32(dst, 0, Mic->L);
-	putUInt32(dst, 4, Mic->R);
-
-	// Reset to the empty message.
-	MichaelClear(Mic);
-}
-
-void MichaelMICFunction(struct michel_mic_t *Mic, u8 *Key,
-			u8 *Data, int Len, u8 priority,
-			u8 *Result)
-{
-	u8 pad_data[4] = { priority, 0, 0, 0 };
-	// Compute the MIC value
-	/*
-	 * IEEE802.11i  page 47
-	 * Figure 43g TKIP MIC processing format
-	 * +--+--+--------+--+----+--+--+--+--+--+--+--+--+
-	 * |6 |6 |1       |3 |M   |1 |1 |1 |1 |1 |1 |1 |1 | Octet
-	 * +--+--+--------+--+----+--+--+--+--+--+--+--+--+
-	 * |DA|SA|Priority|0 |Data|M0|M1|M2|M3|M4|M5|M6|M7|
-	 * +--+--+--------+--+----+--+--+--+--+--+--+--+--+
-	 */
-	MichaelInitializeFunction(Mic, Key);
-	MichaelAppend(Mic, (uint8_t *) Data, 12);	/* |DA|SA| */
-	MichaelAppend(Mic, pad_data, 4);	/* |Priority|0|0|0| */
-	MichaelAppend(Mic, (uint8_t *) (Data + 12), Len - 12);	/* |Data| */
-	MichaelGetMIC(Mic, Result);
-}
diff --git a/drivers/staging/ks7010/michael_mic.h b/drivers/staging/ks7010/michael_mic.h
deleted file mode 100644
index 248f849..0000000
--- a/drivers/staging/ks7010/michael_mic.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- *   Driver for KeyStream wireless LAN
- *
- *   Copyright (C) 2005-2008 KeyStream Corp.
- *   Copyright (C) 2009 Renesas Technology Corp.
- *
- *   This program is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License version 2 as
- *   published by the Free Software Foundation.
- */
-
-/* MichelMIC routine define */
-struct michel_mic_t {
-	u32 K0;	// Key
-	u32 K1;	// Key
-	u32 L;	// Current state
-	u32 R;	// Current state
-	u8 M[4];	// Message accumulator (single word)
-	int nBytesInM;	// # bytes in M
-	u8 Result[8];
-};
-
-void MichaelMICFunction(struct michel_mic_t *Mic, u8 *Key,
-			u8 *Data, int Len, u8 priority,
-			u8 *Result);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] staging: ks7010: remove custom Michael MIC implementation
  2017-03-31  4:47 ` [PATCH RFC] staging: ks7010: " Tobin C. Harding
@ 2017-03-31  5:16   ` Joe Perches
  2017-03-31  9:37     ` Tobin C. Harding
  0 siblings, 1 reply; 19+ messages in thread
From: Joe Perches @ 2017-03-31  5:16 UTC (permalink / raw)
  To: Tobin C. Harding, linux-kernel
  Cc: Greg Kroah-Hartman, Wolfram Sang, devel, Tycho Andersen

On Fri, 2017-03-31 at 15:47 +1100, Tobin C. Harding wrote:
> ks7010 currently uses a custom implementation of the Michael MIC
> algorithm. The kernel has an implementation of this algorithm
> already, we should use it.

ok, trivia:

Do please run your patch through checkpatch and fix a few style nits.

$ ./scripts/checkpatch.pl ~/1.mbox --strict --terse | cut -f2- -d":"
161: WARNING: line over 80 characters
170: WARNING: Missing a blank line after declarations
205: WARNING: line over 80 characters
229: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
263: WARNING: Prefer using "%s", __func__ to embedded function names
264: ERROR: code indent should use tabs where possible
264: WARNING: quoted string split across lines
272: WARNING: Prefer using "%s", __func__ to embedded function names
273: ERROR: code indent should use tabs where possible
273: WARNING: quoted string split across lines
325: WARNING: Prefer pr_warn(... to pr_warning(...
 2 errors, 9 warnings, 0 checks, 262 lines checked

and

> diff --git a/drivers/staging/ks7010/mic.c b/drivers/staging/ks7010/mic.c
[]
> +int ks_wlan_mic(struct crypto_shash *tfm_michael, u8 *key,
> +		u8 priority, u8 *data, size_t data_len, u8 *mic)
> +{
> +	SHASH_DESC_ON_STACK(desc, tfm_michael);
> +	u8 hdr[ETH_HLEN + 2]; /* 16 bytes */

It might be better to declare a struct for this

> +	hdr[ETH_ALEN * 2] = priority;
> +	hdr[ETH_ALEN * 2 + 1] = 0;
> +	hdr[ETH_ALEN * 2 + 2] = 0;
> +	hdr[ETH_ALEN * 2 + 3] = 0;

And use struct members here.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-03-31  4:47 [PATCH RFC] remove custom Michael MIC implementation Tobin C. Harding
  2017-03-31  4:47 ` [PATCH RFC] staging: ks7010: " Tobin C. Harding
@ 2017-03-31  7:58 ` Wolfram Sang
  2017-03-31 10:21   ` Tobin C. Harding
  1 sibling, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2017-03-31  7:58 UTC (permalink / raw)
  To: Tobin C. Harding; +Cc: linux-kernel, Greg Kroah-Hartman, devel, Tycho Andersen

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]


> The code is untested, I have hardware in the mail.

Cool!

> If any one is interested and has any comments I would really like to
> hear them. I am open to all suggestions (even down to trivial coding
> style issues).

I'll just repeat that the key move to get this driver out of staging is
to get away from the WEXT interface to CFG80211. Otherwise no chance
that wireless maintainers will even look at it. This is a huge change
but once it is done, features like Michael MIC come with it for free
(from what I recall, I am not a wireless expert myself).

Without the CFG80211 conversion, replacing the Michael custom
implementation with the in-kernel one makes the driver a tad better and
is good exercise. However, it will sadly not help to get the driver out
of staging.

But if you want a clean WEXT driver first, this is a step in the right
direction.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] staging: ks7010: remove custom Michael MIC implementation
  2017-03-31  5:16   ` Joe Perches
@ 2017-03-31  9:37     ` Tobin C. Harding
  0 siblings, 0 replies; 19+ messages in thread
From: Tobin C. Harding @ 2017-03-31  9:37 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, Greg Kroah-Hartman, Wolfram Sang, devel, Tycho Andersen

On Thu, Mar 30, 2017 at 10:16:05PM -0700, Joe Perches wrote:
> On Fri, 2017-03-31 at 15:47 +1100, Tobin C. Harding wrote:
> > ks7010 currently uses a custom implementation of the Michael MIC
> > algorithm. The kernel has an implementation of this algorithm
> > already, we should use it.
> 
> ok, trivia:
> 
> Do please run your patch through checkpatch and fix a few style nits.
> 
> $ ./scripts/checkpatch.pl ~/1.mbox --strict --terse | cut -f2- -d":"
> 161: WARNING: line over 80 characters
> 170: WARNING: Missing a blank line after declarations
> 205: WARNING: line over 80 characters
> 229: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> 263: WARNING: Prefer using "%s", __func__ to embedded function names
> 264: ERROR: code indent should use tabs where possible
> 264: WARNING: quoted string split across lines
> 272: WARNING: Prefer using "%s", __func__ to embedded function names
> 273: ERROR: code indent should use tabs where possible
> 273: WARNING: quoted string split across lines
> 325: WARNING: Prefer pr_warn(... to pr_warning(...
>  2 errors, 9 warnings, 0 checks, 262 lines checked

Face palm :)

> 
> and
> 
> > diff --git a/drivers/staging/ks7010/mic.c b/drivers/staging/ks7010/mic.c
> []
> > +int ks_wlan_mic(struct crypto_shash *tfm_michael, u8 *key,
> > +		u8 priority, u8 *data, size_t data_len, u8 *mic)
> > +{
> > +	SHASH_DESC_ON_STACK(desc, tfm_michael);
> > +	u8 hdr[ETH_HLEN + 2]; /* 16 bytes */
> 
> It might be better to declare a struct for this
> 
> > +	hdr[ETH_ALEN * 2] = priority;
> > +	hdr[ETH_ALEN * 2 + 1] = 0;
> > +	hdr[ETH_ALEN * 2 + 2] = 0;
> > +	hdr[ETH_ALEN * 2 + 3] = 0;
> 
> And use struct members here.
> 

Thanks Joe,
Tobin.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-03-31  7:58 ` [PATCH RFC] " Wolfram Sang
@ 2017-03-31 10:21   ` Tobin C. Harding
  2017-03-31 10:42     ` Wolfram Sang
  2017-04-03  5:19     ` Kalle Valo
  0 siblings, 2 replies; 19+ messages in thread
From: Tobin C. Harding @ 2017-03-31 10:21 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-kernel, Greg Kroah-Hartman, devel, Tycho Andersen

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

On Fri, Mar 31, 2017 at 09:58:51AM +0200, Wolfram Sang wrote:
> 
> > The code is untested, I have hardware in the mail.
> 
> Cool!

The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
a SDW-823 and should use the ks7010 driver. I am going to attempt to
get it running on a Raspberry Pi B+. I ordered the wrong size break
out board originally so waiting on the new one now.

> 
> > If any one is interested and has any comments I would really like to
> > hear them. I am open to all suggestions (even down to trivial coding
> > style issues).
> 
> I'll just repeat that the key move to get this driver out of staging is
> to get away from the WEXT interface to CFG80211. Otherwise no chance
> that wireless maintainers will even look at it. This is a huge change
> but once it is done, features like Michael MIC come with it for free
> (from what I recall, I am not a wireless expert myself).

That would explain why I could not find more than the Orinoco driver
using the Michael MIC module directly.

> Without the CFG80211 conversion, replacing the Michael custom
> implementation with the in-kernel one makes the driver a tad better and
> is good exercise. However, it will sadly not help to get the driver out
> of staging.

I'll drop it then. Could you please tell me, is there any thing else
more I need to do to let LKML know that this RFC is dropped? Or is
this reply enough. I don't want to use any ones time unnecessarily.

> 
> But if you want a clean WEXT driver first, this is a step in the right
> direction.
> 

Let's go for a CFG80211 driver and get out of staging :) So next step
is I guess study the ath6kl driver, learn how CFG80211 is done and
implement that interface in ks7010? Oh, and test that it works.

thanks,
Tobin.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-03-31 10:21   ` Tobin C. Harding
@ 2017-03-31 10:42     ` Wolfram Sang
  2017-04-01 10:40       ` Tobin C. Harding
  2017-04-26 23:23       ` Tobin C. Harding
  2017-04-03  5:19     ` Kalle Valo
  1 sibling, 2 replies; 19+ messages in thread
From: Wolfram Sang @ 2017-03-31 10:42 UTC (permalink / raw)
  To: Tobin C. Harding; +Cc: linux-kernel, Greg Kroah-Hartman, devel, Tycho Andersen

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

Hi,

> The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
> a SDW-823 and should use the ks7010 driver.

Sorry, likely not. It is an early SDW-821 and has a MediaTek chipset for
which no driver is known:
https://wikidevi.com/wiki/Spectec_SDW-821_%28MediaTek%29

For SDW-821 (SD size) and KS7010, you'd need a "S2Y-WLAN-11G-K" (but I
have never seen one yet):
https://wikidevi.com/wiki/Spectec_SDW-821_%28KeyStream%29

One can find an SDW-823 (microSD) "S2Y-MWLAN-11B-G" once in a while:
https://wikidevi.com/wiki/Spectec_SDW-823

> > Without the CFG80211 conversion, replacing the Michael custom
> > implementation with the in-kernel one makes the driver a tad better and
> > is good exercise. However, it will sadly not help to get the driver out
> > of staging.
> 
> I'll drop it then. Could you please tell me, is there any thing else
> more I need to do to let LKML know that this RFC is dropped? Or is
> this reply enough. I don't want to use any ones time unnecessarily.

That should do.

> Let's go for a CFG80211 driver and get out of staging :) So next step
> is I guess study the ath6kl driver, learn how CFG80211 is done and
> implement that interface in ks7010? Oh, and test that it works.

Yes, have a look around and check if you like that task. I might have a
spare SDW-823 lying around if you are up to it. But check first, it is
not a trivial task. On the pro side, there are tons of interesting
things about WiFI and kernel development to learn on the way :)

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-03-31 10:42     ` Wolfram Sang
@ 2017-04-01 10:40       ` Tobin C. Harding
  2017-04-26 23:23       ` Tobin C. Harding
  1 sibling, 0 replies; 19+ messages in thread
From: Tobin C. Harding @ 2017-04-01 10:40 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-kernel, Greg Kroah-Hartman, devel, Tycho Andersen

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

On Fri, Mar 31, 2017 at 12:42:22PM +0200, Wolfram Sang wrote:
> Hi,
> 
> > The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
> > a SDW-823 and should use the ks7010 driver.
> 
> Sorry, likely not. It is an early SDW-821 and has a MediaTek chipset for
> which no driver is known:
> https://wikidevi.com/wiki/Spectec_SDW-821_%28MediaTek%29

Oh damn.

> 
> For SDW-821 (SD size) and KS7010, you'd need a "S2Y-WLAN-11G-K" (but I
> have never seen one yet):
> https://wikidevi.com/wiki/Spectec_SDW-821_%28KeyStream%29
> 
> One can find an SDW-823 (microSD) "S2Y-MWLAN-11B-G" once in a while:
> https://wikidevi.com/wiki/Spectec_SDW-823

That's actually the page I was reading, I didn't notice the 'M' in there.

> > > Without the CFG80211 conversion, replacing the Michael custom
> > > implementation with the in-kernel one makes the driver a tad better and
> > > is good exercise. However, it will sadly not help to get the driver out
> > > of staging.
> > 
> > I'll drop it then. Could you please tell me, is there any thing else
> > more I need to do to let LKML know that this RFC is dropped? Or is
> > this reply enough. I don't want to use any ones time unnecessarily.
> 
> That should do.
> 
> > Let's go for a CFG80211 driver and get out of staging :) So next step
> > is I guess study the ath6kl driver, learn how CFG80211 is done and
> > implement that interface in ks7010? Oh, and test that it works.
> 
> Yes, have a look around and check if you like that task. I might have a
> spare SDW-823 lying around if you are up to it. But check first, it is
> not a trivial task. On the pro side, there are tons of interesting
> things about WiFI and kernel development to learn on the way :)

I like the task. You had me at 'this is a huge change...' in your
first email. Whether I have the ability is yet to be seen. Thanks for
the offer of the card, let's wait first to see if I can make any
progress that warrants testing. The new breakout board turned up in
the mail today, super glad to read your email before I spent the day
tomorrow trying to get it working :)

> 
> Regards,
> 
>    Wolfram
> 

Cheers Wolfram,
Tobin.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-03-31 10:21   ` Tobin C. Harding
  2017-03-31 10:42     ` Wolfram Sang
@ 2017-04-03  5:19     ` Kalle Valo
  2017-04-03  9:03       ` Tobin C. Harding
  1 sibling, 1 reply; 19+ messages in thread
From: Kalle Valo @ 2017-04-03  5:19 UTC (permalink / raw)
  To: Tobin C. Harding
  Cc: Wolfram Sang, linux-kernel, Greg Kroah-Hartman, devel,
	Tycho Andersen, linux-wireless

+ linux-wireless

"Tobin C. Harding" <me@tobin.cc> writes:

> On Fri, Mar 31, 2017 at 09:58:51AM +0200, Wolfram Sang wrote:
>> 
>> > The code is untested, I have hardware in the mail.
>> 
>> Cool!
>
> The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
> a SDW-823 and should use the ks7010 driver. I am going to attempt to
> get it running on a Raspberry Pi B+. I ordered the wrong size break
> out board originally so waiting on the new one now.
>
>> 
>> > If any one is interested and has any comments I would really like to
>> > hear them. I am open to all suggestions (even down to trivial coding
>> > style issues).
>> 
>> I'll just repeat that the key move to get this driver out of staging is
>> to get away from the WEXT interface to CFG80211. Otherwise no chance
>> that wireless maintainers will even look at it. This is a huge change
>> but once it is done, features like Michael MIC come with it for free
>> (from what I recall, I am not a wireless expert myself).
>
> That would explain why I could not find more than the Orinoco driver
> using the Michael MIC module directly.

I think cfg80211 and mac80211 got mixed up. mac80211 (the full IEEE
802.11 stack for "softmac" devices) provides Michael MIC implementation,
but cfg80211 (for "hardmac" devices) does not.

>> Without the CFG80211 conversion, replacing the Michael custom
>> implementation with the in-kernel one makes the driver a tad better and
>> is good exercise. However, it will sadly not help to get the driver out
>> of staging.
>
> I'll drop it then. Could you please tell me, is there any thing else
> more I need to do to let LKML know that this RFC is dropped? Or is
> this reply enough. I don't want to use any ones time unnecessarily.
>
>> 
>> But if you want a clean WEXT driver first, this is a step in the right
>> direction.
>
> Let's go for a CFG80211 driver and get out of staging :) So next step
> is I guess study the ath6kl driver, learn how CFG80211 is done and
> implement that interface in ks7010? Oh, and test that it works.

Please keep linux-wireless list in loop so that people on that list can
help.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-03  5:19     ` Kalle Valo
@ 2017-04-03  9:03       ` Tobin C. Harding
  2017-04-03  9:50         ` Toke Høiland-Jørgensen
                           ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Tobin C. Harding @ 2017-04-03  9:03 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Wolfram Sang, linux-kernel, Greg Kroah-Hartman, devel,
	Tycho Andersen, linux-wireless

On Mon, Apr 03, 2017 at 08:19:40AM +0300, Kalle Valo wrote:
> + linux-wireless
> 
> "Tobin C. Harding" <me@tobin.cc> writes:
> 
> > On Fri, Mar 31, 2017 at 09:58:51AM +0200, Wolfram Sang wrote:
> >> 
> >> > The code is untested, I have hardware in the mail.
> >> 
> >> Cool!
> >
> > The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
> > a SDW-823 and should use the ks7010 driver. I am going to attempt to
> > get it running on a Raspberry Pi B+. I ordered the wrong size break
> > out board originally so waiting on the new one now.
> >
> >> 
> >> > If any one is interested and has any comments I would really like to
> >> > hear them. I am open to all suggestions (even down to trivial coding
> >> > style issues).
> >> 
> >> I'll just repeat that the key move to get this driver out of staging is
> >> to get away from the WEXT interface to CFG80211. Otherwise no chance
> >> that wireless maintainers will even look at it. This is a huge change
> >> but once it is done, features like Michael MIC come with it for free
> >> (from what I recall, I am not a wireless expert myself).
> >
> > That would explain why I could not find more than the Orinoco driver
> > using the Michael MIC module directly.
> 
> I think cfg80211 and mac80211 got mixed up. mac80211 (the full IEEE
> 802.11 stack for "softmac" devices) provides Michael MIC implementation,
> but cfg80211 (for "hardmac" devices) does not.

Cool, thanks for clarifying. Hilariously I was just sitting down
trying to figure out what was up after a day spent trying to merge
ideas from ath6kl (fullmac) and ks7010. I finally spent some time
reading the cw1200 driver (softmac) for further inspiration.

> >> Without the CFG80211 conversion, replacing the Michael custom
> >> implementation with the in-kernel one makes the driver a tad better and
> >> is good exercise. However, it will sadly not help to get the driver out
> >> of staging.
> >
> > I'll drop it then. Could you please tell me, is there any thing else
> > more I need to do to let LKML know that this RFC is dropped? Or is
> > this reply enough. I don't want to use any ones time unnecessarily.
> >
> >> 
> >> But if you want a clean WEXT driver first, this is a step in the right
> >> direction.
> >
> > Let's go for a CFG80211 driver and get out of staging :) So next step
> > is I guess study the ath6kl driver, learn how CFG80211 is done and
> > implement that interface in ks7010? Oh, and test that it works.
> 
> Please keep linux-wireless list in loop so that people on that list can
> help.

How newbie friendly is the linux-wireless list please? I am having
trouble separating the data path code from the control path. I think I
will spend a few more days on it though before asking any questions.

Except one: do you know off the top of your head of a canonical
implementation of a softmac wi-fi driver.

thanks,
Tobin.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-03  9:03       ` Tobin C. Harding
@ 2017-04-03  9:50         ` Toke Høiland-Jørgensen
  2017-04-03 10:15           ` Arend Van Spriel
  2017-04-03  9:55         ` Kalle Valo
  2017-04-03 12:13         ` Greg Kroah-Hartman
  2 siblings, 1 reply; 19+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-04-03  9:50 UTC (permalink / raw)
  To: Tobin C. Harding
  Cc: Kalle Valo, Tycho Andersen, Wolfram Sang, Greg Kroah-Hartman,
	linux-wireless, linux-kernel, devel

"Tobin C. Harding" <me@tobin.cc> writes:

> Except one: do you know off the top of your head of a canonical
> implementation of a softmac wi-fi driver.

I'll suggest taking a look at the ath9k driver :)

-Toke

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-03  9:03       ` Tobin C. Harding
  2017-04-03  9:50         ` Toke Høiland-Jørgensen
@ 2017-04-03  9:55         ` Kalle Valo
  2017-04-03 12:13         ` Greg Kroah-Hartman
  2 siblings, 0 replies; 19+ messages in thread
From: Kalle Valo @ 2017-04-03  9:55 UTC (permalink / raw)
  To: Tobin C. Harding
  Cc: Wolfram Sang, linux-kernel, Greg Kroah-Hartman, devel,
	Tycho Andersen, linux-wireless

"Tobin C. Harding" <me@tobin.cc> writes:

>> >> But if you want a clean WEXT driver first, this is a step in the right
>> >> direction.
>> >
>> > Let's go for a CFG80211 driver and get out of staging :) So next step
>> > is I guess study the ath6kl driver, learn how CFG80211 is done and
>> > implement that interface in ks7010? Oh, and test that it works.
>> 
>> Please keep linux-wireless list in loop so that people on that list can
>> help.
>
> How newbie friendly is the linux-wireless list please?

I would claim quite friendly, but I'm finnish and our definition of
"friendly" seems to be very different from rest of the world ;)

> I am having trouble separating the data path code from the control
> path. I think I will spend a few more days on it though before asking
> any questions.

People do send questions to the list and most of the time they get
answered. So go for it.

There's also an irc channel which usually is helpful:

https://wireless.wiki.kernel.org/en/users/support#linux_wireless_user_irc_channel

> Except one: do you know off the top of your head of a canonical
> implementation of a softmac wi-fi driver.

You mean a mac80211 driver? mac80211_hwsim is the simplest one, but
that's not a real driver as it's a simulator. Just grep
ieee80211_register() to find all the drivers. Smaller drivers like
rtl8xxxu or wl1251 might be good starting points to get familiar with
the stack.

And I guess you already saw the documentation:

https://wireless.wiki.kernel.org/en/developers/documentation/mac80211

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-03  9:50         ` Toke Høiland-Jørgensen
@ 2017-04-03 10:15           ` Arend Van Spriel
  2017-04-03 21:39             ` Tobin C. Harding
  0 siblings, 1 reply; 19+ messages in thread
From: Arend Van Spriel @ 2017-04-03 10:15 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Tobin C. Harding
  Cc: Kalle Valo, Tycho Andersen, Wolfram Sang, Greg Kroah-Hartman,
	linux-wireless, linux-kernel, devel

seems we are missing out again?

On 3-4-2017 11:50, Toke Høiland-Jørgensen wrote:
> "Tobin C. Harding" <me@tobin.cc> writes:
> 
>> Except one: do you know off the top of your head of a canonical
>> implementation of a softmac wi-fi driver.
> 
> I'll suggest taking a look at the ath9k driver :)

Looking at ks7010 driver it looks like it has 802.11 stack in firmware
and not sure if Renesas is actively supporting this effort to come up
with mac80211-friendly firmware or provide detailed chip info.

Regards,
Arend

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-03  9:03       ` Tobin C. Harding
  2017-04-03  9:50         ` Toke Høiland-Jørgensen
  2017-04-03  9:55         ` Kalle Valo
@ 2017-04-03 12:13         ` Greg Kroah-Hartman
  2 siblings, 0 replies; 19+ messages in thread
From: Greg Kroah-Hartman @ 2017-04-03 12:13 UTC (permalink / raw)
  To: Tobin C. Harding
  Cc: Kalle Valo, Tycho Andersen, Wolfram Sang, linux-wireless,
	linux-kernel, devel

On Mon, Apr 03, 2017 at 07:03:59PM +1000, Tobin C. Harding wrote:
> On Mon, Apr 03, 2017 at 08:19:40AM +0300, Kalle Valo wrote:
> > + linux-wireless
> > 
> > "Tobin C. Harding" <me@tobin.cc> writes:
> > 
> > > On Fri, Mar 31, 2017 at 09:58:51AM +0200, Wolfram Sang wrote:
> > >> 
> > >> > The code is untested, I have hardware in the mail.
> > >> 
> > >> Cool!
> > >
> > > The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
> > > a SDW-823 and should use the ks7010 driver. I am going to attempt to
> > > get it running on a Raspberry Pi B+. I ordered the wrong size break
> > > out board originally so waiting on the new one now.
> > >
> > >> 
> > >> > If any one is interested and has any comments I would really like to
> > >> > hear them. I am open to all suggestions (even down to trivial coding
> > >> > style issues).
> > >> 
> > >> I'll just repeat that the key move to get this driver out of staging is
> > >> to get away from the WEXT interface to CFG80211. Otherwise no chance
> > >> that wireless maintainers will even look at it. This is a huge change
> > >> but once it is done, features like Michael MIC come with it for free
> > >> (from what I recall, I am not a wireless expert myself).
> > >
> > > That would explain why I could not find more than the Orinoco driver
> > > using the Michael MIC module directly.
> > 
> > I think cfg80211 and mac80211 got mixed up. mac80211 (the full IEEE
> > 802.11 stack for "softmac" devices) provides Michael MIC implementation,
> > but cfg80211 (for "hardmac" devices) does not.
> 
> Cool, thanks for clarifying. Hilariously I was just sitting down
> trying to figure out what was up after a day spent trying to merge
> ideas from ath6kl (fullmac) and ks7010. I finally spent some time
> reading the cw1200 driver (softmac) for further inspiration.

As an example of a driver that has been moved from an internal wireless
stack to using the kernel stack is the vt6655 driver.  I think it's the
only driver that has done this type of transition, so look at the patch
history of it for an example of what to do.

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-03 10:15           ` Arend Van Spriel
@ 2017-04-03 21:39             ` Tobin C. Harding
  2017-04-04 21:31               ` Arend Van Spriel
  0 siblings, 1 reply; 19+ messages in thread
From: Tobin C. Harding @ 2017-04-03 21:39 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Toke Høiland-Jørgensen, Kalle Valo, Tycho Andersen,
	Wolfram Sang, Greg Kroah-Hartman, linux-wireless, linux-kernel,
	devel

On Mon, Apr 03, 2017 at 12:15:15PM +0200, Arend Van Spriel wrote:
> seems we are missing out again?

Sorry, I don't understand what this comment means?

> On 3-4-2017 11:50, Toke Høiland-Jørgensen wrote:
> > "Tobin C. Harding" <me@tobin.cc> writes:
> > 
> >> Except one: do you know off the top of your head of a canonical
> >> implementation of a softmac wi-fi driver.
> > 
> > I'll suggest taking a look at the ath9k driver :)
> 
> Looking at ks7010 driver it looks like it has 802.11 stack in firmware
> and not sure if Renesas is actively supporting this effort to come up
> with mac80211-friendly firmware or provide detailed chip info.

Thanks for taking a look. If the ks7010 driver has 802.11 stack in
firmware does that mean it is not compatible with using the kernel
mac80211 stack with the current firmware? I do not have my hopes up
about getting any chip information out of Renesas, all I think we have
to go in is the current WEXT driver.

This is an exercise in learning for me, but I do not want to take any
ones time up with a project that is not that useful. Is getting the
ks7010 driver out of staging something that is of use to the kernel
community or are there other wi-fi tasks that our time is better spent
on?

thanks,
Tobin.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-03 21:39             ` Tobin C. Harding
@ 2017-04-04 21:31               ` Arend Van Spriel
  2017-04-05  1:18                 ` Tobin C. Harding
  0 siblings, 1 reply; 19+ messages in thread
From: Arend Van Spriel @ 2017-04-04 21:31 UTC (permalink / raw)
  To: Tobin C. Harding
  Cc: Toke Høiland-Jørgensen, Kalle Valo, Tycho Andersen,
	Wolfram Sang, Greg Kroah-Hartman, linux-wireless, linux-kernel,
	devel



On 3-4-2017 23:39, Tobin C. Harding wrote:
> On Mon, Apr 03, 2017 at 12:15:15PM +0200, Arend Van Spriel wrote:
>> seems we are missing out again?
> 
> Sorry, I don't understand what this comment means?

My bad. I had to reset my email account in thunderbird and now it needs
to learn anew what is spam and what is not. So I missed out on the
conversation as some messages (a lot actually) ended up in my spam folder.

>> On 3-4-2017 11:50, Toke Høiland-Jørgensen wrote:
>>> "Tobin C. Harding" <me@tobin.cc> writes:
>>>
>>>> Except one: do you know off the top of your head of a canonical
>>>> implementation of a softmac wi-fi driver.
>>>
>>> I'll suggest taking a look at the ath9k driver :)
>>
>> Looking at ks7010 driver it looks like it has 802.11 stack in firmware
>> and not sure if Renesas is actively supporting this effort to come up
>> with mac80211-friendly firmware or provide detailed chip info.
> 
> Thanks for taking a look. If the ks7010 driver has 802.11 stack in
> firmware does that mean it is not compatible with using the kernel
> mac80211 stack with the current firmware? I do not have my hopes up
> about getting any chip information out of Renesas, all I think we have
> to go in is the current WEXT driver.

Indeed. That is my gut feeling as well and if that is truly the case
your best option would be a cfg80211-based driver like ath6kl, mwifiex,
and brcmfmac.

> This is an exercise in learning for me, but I do not want to take any
> ones time up with a project that is not that useful. Is getting the
> ks7010 driver out of staging something that is of use to the kernel
> community or are there other wi-fi tasks that our time is better spent
> on?

If there are linux users with this hardware than sure. Especially if it
revised to interface with the latest wireless subsystem so tools like iw
can operate on it.

Also I suppose it is preferred if a driver is maintained. I do not see
the ks7010 listed in the MAINTAINERS file yet.

> thanks,
> Tobin.
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-04 21:31               ` Arend Van Spriel
@ 2017-04-05  1:18                 ` Tobin C. Harding
  0 siblings, 0 replies; 19+ messages in thread
From: Tobin C. Harding @ 2017-04-05  1:18 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Toke Høiland-Jørgensen, Kalle Valo, Tycho Andersen,
	Wolfram Sang, Greg Kroah-Hartman, linux-wireless, linux-kernel,
	devel

On Tue, Apr 04, 2017 at 11:31:14PM +0200, Arend Van Spriel wrote:
> >> On 3-4-2017 11:50, Toke Høiland-Jørgensen wrote:
> >>> "Tobin C. Harding" <me@tobin.cc> writes:
> >>>
> >>>> Except one: do you know off the top of your head of a canonical
> >>>> implementation of a softmac wi-fi driver.
> >>>
> >>> I'll suggest taking a look at the ath9k driver :)
> >>
> >> Looking at ks7010 driver it looks like it has 802.11 stack in firmware
> >> and not sure if Renesas is actively supporting this effort to come up
> >> with mac80211-friendly firmware or provide detailed chip info.
> > 
> > Thanks for taking a look. If the ks7010 driver has 802.11 stack in
> > firmware does that mean it is not compatible with using the kernel
> > mac80211 stack with the current firmware? I do not have my hopes up
> > about getting any chip information out of Renesas, all I think we have
> > to go in is the current WEXT driver.
> 
> Indeed. That is my gut feeling as well and if that is truly the case
> your best option would be a cfg80211-based driver like ath6kl, mwifiex,
> and brcmfmac.

Ok. So that would mean that we still have to do the Michael MIC in software
(using kernel crypto API) right?

I think the best course of action is to totally clean up the WEXT
interface, including getting Michael MIC working with kernel
crypto, test it all, and then attempt the migration to cfg80211
interface.

If no one thinks that's a bad idea I'll keep working on the WEXT
interface for now.

> > This is an exercise in learning for me, but I do not want to take any
> > ones time up with a project that is not that useful. Is getting the
> > ks7010 driver out of staging something that is of use to the kernel
> > community or are there other wi-fi tasks that our time is better spent
> > on?
> 
> If there are linux users with this hardware than sure. Especially if it
> revised to interface with the latest wireless subsystem so tools like iw
> can operate on it.
> 
> Also I suppose it is preferred if a driver is maintained. I do not see
> the ks7010 listed in the MAINTAINERS file yet.

How's this for a MAINTAINERS entry?

KS7010 KEYSTREAM DRIVER
M:      Wolfram Sang <wsa@the-dreams.de>
M:      Tobin Harding <me@tobin.cc>
L:      driverdev-devel@linuxdriverproject.org
S:      Maintained
F:      drivers/staging/ks7010/

Wolfram, is this ok by you;
(a) To put your name
(b) To put my name

Is the status correct, I'm happy to help maintain it if that is acceptable
for someone of my experience level to do so. Is this driver part of
your job?

thanks,
Tobin.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-03-31 10:42     ` Wolfram Sang
  2017-04-01 10:40       ` Tobin C. Harding
@ 2017-04-26 23:23       ` Tobin C. Harding
  2017-04-28  6:09         ` Wolfram Sang
  1 sibling, 1 reply; 19+ messages in thread
From: Tobin C. Harding @ 2017-04-26 23:23 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-kernel, Greg Kroah-Hartman, devel, Tycho Andersen

[-- Attachment #1: Type: text/plain, Size: 2963 bytes --]

On Fri, Mar 31, 2017 at 12:42:22PM +0200, Wolfram Sang wrote:
> Hi,
> 
> > The card I have is a Spectec FCC ID: S2Y-WLAN-11B-G which I believe is
> > a SDW-823 and should use the ks7010 driver.
> 
> Sorry, likely not. It is an early SDW-821 and has a MediaTek chipset for
> which no driver is known:
> https://wikidevi.com/wiki/Spectec_SDW-821_%28MediaTek%29
> 
> For SDW-821 (SD size) and KS7010, you'd need a "S2Y-WLAN-11G-K" (but I
> have never seen one yet):
> https://wikidevi.com/wiki/Spectec_SDW-821_%28KeyStream%29
> 
> One can find an SDW-823 (microSD) "S2Y-MWLAN-11B-G" once in a while:
> https://wikidevi.com/wiki/Spectec_SDW-823
> 
> > > Without the CFG80211 conversion, replacing the Michael custom
> > > implementation with the in-kernel one makes the driver a tad better and
> > > is good exercise. However, it will sadly not help to get the driver out
> > > of staging.
> > 
> > I'll drop it then. Could you please tell me, is there any thing else
> > more I need to do to let LKML know that this RFC is dropped? Or is
> > this reply enough. I don't want to use any ones time unnecessarily.
> 
> That should do.
> 
> > Let's go for a CFG80211 driver and get out of staging :) So next step
> > is I guess study the ath6kl driver, learn how CFG80211 is done and
> > implement that interface in ks7010? Oh, and test that it works.
> 
> Yes, have a look around and check if you like that task. I might have a
> spare SDW-823 lying around if you are up to it. But check first, it is
> not a trivial task. On the pro side, there are tons of interesting
> things about WiFI and kernel development to learn on the way :)

Hi Wolfram,

After receiving this email a while back I spent a day or two trying to
get started on the cfg80211 conversion. I was unable to make progress
so I decided to spend some time cleaning up the WEXT interface and
learning more about the driver (and wifi in general). As you may have
seen, a series was just merged refactoring the SDIO code, and there
is a series in flight refactoring the host interface code. I started
working on ks_wlan_net.c and will likely put together a patch set next
week if the hostif series gets merged. 

At this stage, I think I have a fair handle on how the ks7010 driver
functions. I have two ideas on how to continue. First, I think it may
be time to do some testing and fix all of the bugs I have introduced
over this last month :). This is the reason for this email. Can I
please take you up on the offer of the SDW-823 card, if you can find
it? Secondly, I think I should spend some time studying the mwifiex
(SoftMAC) and ath6kl (FullMAC) drivers to get a handle on cfg80211.

Then I can have another crack at the ks7010 cfg80211 conversion.

Do you have any other data on the chipset? Have you had any
interaction with Renesas?

Sorry to write you such a long email. Thanks for taking the time to
read it.

Regards,
Tobin.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH RFC] remove custom Michael MIC implementation
  2017-04-26 23:23       ` Tobin C. Harding
@ 2017-04-28  6:09         ` Wolfram Sang
  0 siblings, 0 replies; 19+ messages in thread
From: Wolfram Sang @ 2017-04-28  6:09 UTC (permalink / raw)
  To: Tobin C. Harding; +Cc: linux-kernel, Greg Kroah-Hartman, devel, Tycho Andersen

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

Hi Tobin,

> over this last month :). This is the reason for this email. Can I
> please take you up on the offer of the SDW-823 card, if you can find
> it?

Sure, I have it here. Just mail me (privately) the address to send it
to and I'll ship right away.

> Secondly, I think I should spend some time studying the mwifiex
> (SoftMAC) and ath6kl (FullMAC) drivers to get a handle on cfg80211.

That sounds like a good plan. Great progress you made, really!

> Do you have any other data on the chipset? Have you had any
> interaction with Renesas?

Nope, sadly not. I do work for Renesas as a contractor, but it is a huge
company and I work for a totally different section (SoCs). Plus, it is 8
years old data and it might be only available in Japanese, so I wouldn't
have high hopes. Any specific information you are missing?

All the best,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-04-28  6:09 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-31  4:47 [PATCH RFC] remove custom Michael MIC implementation Tobin C. Harding
2017-03-31  4:47 ` [PATCH RFC] staging: ks7010: " Tobin C. Harding
2017-03-31  5:16   ` Joe Perches
2017-03-31  9:37     ` Tobin C. Harding
2017-03-31  7:58 ` [PATCH RFC] " Wolfram Sang
2017-03-31 10:21   ` Tobin C. Harding
2017-03-31 10:42     ` Wolfram Sang
2017-04-01 10:40       ` Tobin C. Harding
2017-04-26 23:23       ` Tobin C. Harding
2017-04-28  6:09         ` Wolfram Sang
2017-04-03  5:19     ` Kalle Valo
2017-04-03  9:03       ` Tobin C. Harding
2017-04-03  9:50         ` Toke Høiland-Jørgensen
2017-04-03 10:15           ` Arend Van Spriel
2017-04-03 21:39             ` Tobin C. Harding
2017-04-04 21:31               ` Arend Van Spriel
2017-04-05  1:18                 ` Tobin C. Harding
2017-04-03  9:55         ` Kalle Valo
2017-04-03 12:13         ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.