Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1738108 > unrolled thread

[PATCH] mac80211: aead api to reduce redundancy

Back to article view | Back to linux.kernel

Contents

  [PATCH] mac80211: aead api to reduce redundancy Xiang Gao <qasdfgtyuiop@gmail.com> - 2017-09-24 07:50 +0200
    Re: [PATCH] mac80211: aead api to reduce redundancy Johannes Berg <johannes@sipsolutions.net> - 2017-09-24 17:10 +0200
      Re: [PATCH] mac80211: aead api to reduce redundancy Xiang Gao <qasdfgtyuiop@gmail.com> - 2017-09-24 19:30 +0200
        Re: [PATCH] mac80211: aead api to reduce redundancy Johannes Berg <johannes@sipsolutions.net> - 2017-09-24 19:50 +0200
          Re: [PATCH] mac80211: aead api to reduce redundancy Xiang Gao <qasdfgtyuiop@gmail.com> - 2017-09-24 20:50 +0200
          Re: [PATCH] mac80211: aead api to reduce redundancy Herbert Xu <herbert@gondor.apana.org.au> - 2017-09-25 07:00 +0200
            Re: [PATCH] mac80211: aead api to reduce redundancy Johannes Berg <johannes@sipsolutions.net> - 2017-09-25 07:30 +0200
              Re: [PATCH] mac80211: aead api to reduce redundancy Herbert Xu <herbert@gondor.apana.org.au> - 2017-09-25 08:20 +0200

#1738108 — [PATCH] mac80211: aead api to reduce redundancy

Currently, the aes_ccm.c and aes_gcm.c are almost line by line
copy of each other. This patch reduce code redundancy by moving
the code in these two files to crypto/aead_api.c to make it a
higher level aead api. The aes_ccm.c and aes_gcm.c are removed
and all the functions are now implemented in their headers using
the newly added aead api.

Signed-off-by: Xiang Gao <qasdfgtyuiop@gmail.com>
---
 crypto/Makefile                             |   2 +-
 net/mac80211/aes_gcm.c => crypto/aead_api.c |  53 +++++++------
 include/crypto/aead_api.h                   |  21 +++++
 net/mac80211/Makefile                       |   2 -
 net/mac80211/aes_ccm.c                      | 115 ----------------------------
 net/mac80211/aes_ccm.h                      |  42 +++++++---
 net/mac80211/aes_gcm.h                      |  40 ++++++++--
 net/mac80211/key.c                          |   1 +
 net/mac80211/wpa.c                          |  11 +--
 9 files changed, 118 insertions(+), 169 deletions(-)
 rename net/mac80211/aes_gcm.c => crypto/aead_api.c (54%)
 create mode 100644 include/crypto/aead_api.h
 delete mode 100644 net/mac80211/aes_ccm.c

diff --git a/crypto/Makefile b/crypto/Makefile
index d41f0331b085..541316db5841 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -14,7 +14,7 @@ crypto_algapi-$(CONFIG_PROC_FS) += proc.o
 crypto_algapi-y := algapi.o scatterwalk.o $(crypto_algapi-y)
 obj-$(CONFIG_CRYPTO_ALGAPI2) += crypto_algapi.o
 
-obj-$(CONFIG_CRYPTO_AEAD2) += aead.o
+obj-$(CONFIG_CRYPTO_AEAD2) += aead.o aead_api.o
 
 crypto_blkcipher-y := ablkcipher.o
 crypto_blkcipher-y += blkcipher.o
diff --git a/net/mac80211/aes_gcm.c b/crypto/aead_api.c
similarity index 54%
rename from net/mac80211/aes_gcm.c
rename to crypto/aead_api.c
index 8a4397cc1b08..9585ee400b2e 100644
--- a/net/mac80211/aes_gcm.c
+++ b/crypto/aead_api.c
@@ -1,7 +1,4 @@
-/*
- * Copyright 2014-2015, Qualcomm Atheros, Inc.
- *
- * This program is free software; you can redistribute it and/or modify
+/* 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.
  */
@@ -9,43 +6,43 @@
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/err.h>
-#include <crypto/aead.h>
+#include <linux/scatterlist.h>
 
-#include <net/mac80211.h>
-#include "key.h"
-#include "aes_gcm.h"
+#include <crypto/aead_api.h>
 
-int ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic)
+int aead_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, size_t aad_len,
+				 u8 *data, size_t data_len, u8 *auth)
 {
 	struct scatterlist sg[3];
 	struct aead_request *aead_req;
 	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
 	u8 *__aad;
 
-	aead_req = kzalloc(reqsize + GCM_AAD_LEN, GFP_ATOMIC);
+	aead_req = kzalloc(reqsize + aad_len, GFP_ATOMIC);
 	if (!aead_req)
 		return -ENOMEM;
 
 	__aad = (u8 *)aead_req + reqsize;
-	memcpy(__aad, aad, GCM_AAD_LEN);
+	memcpy(__aad, aad, aad_len);
 
 	sg_init_table(sg, 3);
-	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
+	sg_set_buf(&sg[0], __aad, aad_len);
 	sg_set_buf(&sg[1], data, data_len);
-	sg_set_buf(&sg[2], mic, IEEE80211_GCMP_MIC_LEN);
+	sg_set_buf(&sg[2], auth, tfm->authsize);
 
 	aead_request_set_tfm(aead_req, tfm);
-	aead_request_set_crypt(aead_req, sg, sg, data_len, j_0);
+	aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
 	aead_request_set_ad(aead_req, sg[0].length);
 
 	crypto_aead_encrypt(aead_req);
 	kzfree(aead_req);
+
 	return 0;
 }
+EXPORT_SYMBOL_GPL(aead_encrypt);
 
-int ieee80211_aes_gcm_decrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic)
+int aead_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, size_t aad_len,
+				 u8 *data, size_t data_len, u8 *auth)
 {
 	struct scatterlist sg[3];
 	struct aead_request *aead_req;
@@ -56,21 +53,20 @@ int ieee80211_aes_gcm_decrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
 	if (data_len == 0)
 		return -EINVAL;
 
-	aead_req = kzalloc(reqsize + GCM_AAD_LEN, GFP_ATOMIC);
+	aead_req = kzalloc(reqsize + aad_len, GFP_ATOMIC);
 	if (!aead_req)
 		return -ENOMEM;
 
 	__aad = (u8 *)aead_req + reqsize;
-	memcpy(__aad, aad, GCM_AAD_LEN);
+	memcpy(__aad, aad, aad_len);
 
 	sg_init_table(sg, 3);
 	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
 	sg_set_buf(&sg[1], data, data_len);
-	sg_set_buf(&sg[2], mic, IEEE80211_GCMP_MIC_LEN);
+	sg_set_buf(&sg[2], auth, tfm->authsize);
 
 	aead_request_set_tfm(aead_req, tfm);
-	aead_request_set_crypt(aead_req, sg, sg,
-			       data_len + IEEE80211_GCMP_MIC_LEN, j_0);
+	aead_request_set_crypt(aead_req, sg, sg, data_len + tfm->authsize, b_0);
 	aead_request_set_ad(aead_req, sg[0].length);
 
 	err = crypto_aead_decrypt(aead_req);
@@ -78,21 +74,22 @@ int ieee80211_aes_gcm_decrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
 
 	return err;
 }
+EXPORT_SYMBOL_GPL(aead_decrypt);
 
-struct crypto_aead *ieee80211_aes_gcm_key_setup_encrypt(const u8 key[],
-							size_t key_len)
+struct crypto_aead *aead_key_setup_encrypt(const char *alg,
+				const u8 key[], size_t key_len, size_t authsize)
 {
 	struct crypto_aead *tfm;
 	int err;
 
-	tfm = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
+	tfm = crypto_alloc_aead(alg, 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(tfm))
 		return tfm;
 
 	err = crypto_aead_setkey(tfm, key, key_len);
 	if (err)
 		goto free_aead;
-	err = crypto_aead_setauthsize(tfm, IEEE80211_GCMP_MIC_LEN);
+	err = crypto_aead_setauthsize(tfm, authsize);
 	if (err)
 		goto free_aead;
 
@@ -102,8 +99,10 @@ struct crypto_aead *ieee80211_aes_gcm_key_setup_encrypt(const u8 key[],
 	crypto_free_aead(tfm);
 	return ERR_PTR(err);
 }
+EXPORT_SYMBOL_GPL(aead_key_setup_encrypt);
 
-void ieee80211_aes_gcm_key_free(struct crypto_aead *tfm)
+void aead_key_free(struct crypto_aead *tfm)
 {
 	crypto_free_aead(tfm);
 }
+EXPORT_SYMBOL_GPL(aead_key_free);
diff --git a/include/crypto/aead_api.h b/include/crypto/aead_api.h
new file mode 100644
index 000000000000..45deda0d538f
--- /dev/null
+++ b/include/crypto/aead_api.h
@@ -0,0 +1,21 @@
+/* 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 _AEAD_API_H
+#define _AEAD_API_H
+
+#include <crypto/aead.h>
+#include <linux/crypto.h>
+
+struct crypto_aead *aead_key_setup_encrypt(const char *alg,
+			const u8 key[], size_t key_len, size_t authsize);
+
+int aead_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
+			size_t aad_len, u8 *data, size_t data_len, u8 *auth);
+int aead_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
+			size_t aad_len, u8 *data, size_t data_len, u8 *auth);
+void aead_key_free(struct crypto_aead *tfm);
+
+#endif /* _AEAD_API_H */
diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index 282912245938..f1efc8c81ce6 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -15,8 +15,6 @@ mac80211-y := \
 	rate.o \
 	michael.o \
 	tkip.o \
-	aes_ccm.o \
-	aes_gcm.o \
 	aes_cmac.o \
 	aes_gmac.o \
 	fils_aead.o \
diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
deleted file mode 100644
index a4e0d59a40dd..000000000000
--- a/net/mac80211/aes_ccm.c
+++ /dev/null
@@ -1,115 +0,0 @@
-/*
- * Copyright 2003-2004, Instant802 Networks, Inc.
- * Copyright 2005-2006, Devicescape Software, Inc.
- *
- * Rewrite: Copyright (C) 2013 Linaro Ltd <ard.biesheuvel@linaro.org>
- *
- * 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/kernel.h>
-#include <linux/types.h>
-#include <linux/err.h>
-#include <crypto/aead.h>
-
-#include <net/mac80211.h>
-#include "key.h"
-#include "aes_ccm.h"
-
-int ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic,
-			      size_t mic_len)
-{
-	struct scatterlist sg[3];
-	struct aead_request *aead_req;
-	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
-	u8 *__aad;
-
-	aead_req = kzalloc(reqsize + CCM_AAD_LEN, GFP_ATOMIC);
-	if (!aead_req)
-		return -ENOMEM;
-
-	__aad = (u8 *)aead_req + reqsize;
-	memcpy(__aad, aad, CCM_AAD_LEN);
-
-	sg_init_table(sg, 3);
-	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
-	sg_set_buf(&sg[1], data, data_len);
-	sg_set_buf(&sg[2], mic, mic_len);
-
-	aead_request_set_tfm(aead_req, tfm);
-	aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
-	aead_request_set_ad(aead_req, sg[0].length);
-
-	crypto_aead_encrypt(aead_req);
-	kzfree(aead_req);
-
-	return 0;
-}
-
-int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic,
-			      size_t mic_len)
-{
-	struct scatterlist sg[3];
-	struct aead_request *aead_req;
-	int reqsize = sizeof(*aead_req) + crypto_aead_reqsize(tfm);
-	u8 *__aad;
-	int err;
-
-	if (data_len == 0)
-		return -EINVAL;
-
-	aead_req = kzalloc(reqsize + CCM_AAD_LEN, GFP_ATOMIC);
-	if (!aead_req)
-		return -ENOMEM;
-
-	__aad = (u8 *)aead_req + reqsize;
-	memcpy(__aad, aad, CCM_AAD_LEN);
-
-	sg_init_table(sg, 3);
-	sg_set_buf(&sg[0], &__aad[2], be16_to_cpup((__be16 *)__aad));
-	sg_set_buf(&sg[1], data, data_len);
-	sg_set_buf(&sg[2], mic, mic_len);
-
-	aead_request_set_tfm(aead_req, tfm);
-	aead_request_set_crypt(aead_req, sg, sg, data_len + mic_len, b_0);
-	aead_request_set_ad(aead_req, sg[0].length);
-
-	err = crypto_aead_decrypt(aead_req);
-	kzfree(aead_req);
-
-	return err;
-}
-
-struct crypto_aead *ieee80211_aes_key_setup_encrypt(const u8 key[],
-						    size_t key_len,
-						    size_t mic_len)
-{
-	struct crypto_aead *tfm;
-	int err;
-
-	tfm = crypto_alloc_aead("ccm(aes)", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(tfm))
-		return tfm;
-
-	err = crypto_aead_setkey(tfm, key, key_len);
-	if (err)
-		goto free_aead;
-	err = crypto_aead_setauthsize(tfm, mic_len);
-	if (err)
-		goto free_aead;
-
-	return tfm;
-
-free_aead:
-	crypto_free_aead(tfm);
-	return ERR_PTR(err);
-}
-
-void ieee80211_aes_key_free(struct crypto_aead *tfm)
-{
-	crypto_free_aead(tfm);
-}
diff --git a/net/mac80211/aes_ccm.h b/net/mac80211/aes_ccm.h
index fcd3254c5cf0..b51ef23201c1 100644
--- a/net/mac80211/aes_ccm.h
+++ b/net/mac80211/aes_ccm.h
@@ -10,19 +10,39 @@
 #ifndef AES_CCM_H
 #define AES_CCM_H
 
-#include <linux/crypto.h>
+#include <crypto/aead_api.h>
 
 #define CCM_AAD_LEN	32
 
-struct crypto_aead *ieee80211_aes_key_setup_encrypt(const u8 key[],
-						    size_t key_len,
-						    size_t mic_len);
-int ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic,
-			      size_t mic_len);
-int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic,
-			      size_t mic_len);
-void ieee80211_aes_key_free(struct crypto_aead *tfm);
+static inline struct crypto_aead *ieee80211_aes_key_setup_encrypt(
+				const u8 key[], size_t key_len, size_t mic_len)
+{
+	return aead_key_setup_encrypt("ccm(aes)", key, key_len, mic_len);
+}
+
+static inline int ieee80211_aes_ccm_encrypt(
+		struct crypto_aead *tfm,
+		u8 *b_0, u8 *aad, u8 *data,
+		size_t data_len, u8 *mic)
+{
+	return aead_encrypt(tfm, b_0, aad + 2,
+						be16_to_cpup((__be16 *)aad),
+						data, data_len, mic);
+}
+
+static inline int ieee80211_aes_ccm_decrypt(
+		struct crypto_aead *tfm,
+		u8 *b_0, u8 *aad, u8 *data,
+		size_t data_len, u8 *mic)
+{
+	return aead_decrypt(tfm, b_0, aad + 2,
+						be16_to_cpup((__be16 *)aad),
+						data, data_len, mic);
+}
+
+static inline void ieee80211_aes_key_free(struct crypto_aead *tfm)
+{
+	return aead_key_free(tfm);
+}
 
 #endif /* AES_CCM_H */
diff --git a/net/mac80211/aes_gcm.h b/net/mac80211/aes_gcm.h
index 55aed5352494..f07359d5ebbd 100644
--- a/net/mac80211/aes_gcm.h
+++ b/net/mac80211/aes_gcm.h
@@ -9,16 +9,40 @@
 #ifndef AES_GCM_H
 #define AES_GCM_H
 
-#include <linux/crypto.h>
+#include <crypto/aead_api.h>
 
 #define GCM_AAD_LEN	32
 
-int ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic);
-int ieee80211_aes_gcm_decrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
-			      u8 *data, size_t data_len, u8 *mic);
-struct crypto_aead *ieee80211_aes_gcm_key_setup_encrypt(const u8 key[],
-							size_t key_len);
-void ieee80211_aes_gcm_key_free(struct crypto_aead *tfm);
+static inline int ieee80211_aes_gcm_encrypt(
+		struct crypto_aead *tfm,
+		u8 *j_0, u8 *aad,  u8 *data,
+		size_t data_len, u8 *mic)
+{
+	return aead_encrypt(tfm, j_0, aad + 2,
+						be16_to_cpup((__be16 *)aad),
+						data, data_len, mic);
+}
+
+static inline int ieee80211_aes_gcm_decrypt(
+		struct crypto_aead *tfm,
+		u8 *j_0, u8 *aad, u8 *data,
+		size_t data_len, u8 *mic)
+{
+	return aead_decrypt(tfm, j_0, aad + 2,
+						be16_to_cpup((__be16 *)aad),
+						data, data_len, mic);
+}
+
+static inline struct crypto_aead *ieee80211_aes_gcm_key_setup_encrypt(
+		const u8 key[], size_t key_len)
+{
+	return aead_key_setup_encrypt("gcm(aes)", key,
+				key_len, IEEE80211_GCMP_MIC_LEN);
+}
+
+static inline void ieee80211_aes_gcm_key_free(struct crypto_aead *tfm)
+{
+	return aead_key_free(tfm);
+}
 
 #endif /* AES_GCM_H */
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index a98fc2b5e0dc..a61cecd078bb 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -18,6 +18,7 @@
 #include <linux/rtnetlink.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/crypto.h>
 #include <net/mac80211.h>
 #include <asm/unaligned.h>
 #include "ieee80211_i.h"
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 0d722ea98a1b..e30e09c1e149 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -464,7 +464,7 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
 	pos += IEEE80211_CCMP_HDR_LEN;
 	ccmp_special_blocks(skb, pn, b_0, aad);
 	return ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, b_0, aad, pos, len,
-					 skb_put(skb, mic_len), mic_len);
+				skb_put(skb, key->u.ccmp.tfm->authsize));
 }
 
 
@@ -540,10 +540,11 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
 			ccmp_special_blocks(skb, pn, b_0, aad);
 
 			if (ieee80211_aes_ccm_decrypt(
-				    key->u.ccmp.tfm, b_0, aad,
-				    skb->data + hdrlen + IEEE80211_CCMP_HDR_LEN,
-				    data_len,
-				    skb->data + skb->len - mic_len, mic_len))
+				key->u.ccmp.tfm, b_0, aad,
+				skb->data + hdrlen + IEEE80211_CCMP_HDR_LEN,
+				data_len,
+				skb->data + skb->len - key->u.ccmp.tfm->authsize
+			))
 				return RX_DROP_UNUSABLE;
 		}
 
-- 
2.14.1

[toc] | [next] | [standalone]

#1738209

On Sun, 2017-09-24 at 01:40 -0400, Xiang Gao wrote:
> Currently, the aes_ccm.c and aes_gcm.c are almost line by line
> copy of each other. This patch reduce code redundancy by moving
> the code in these two files to crypto/aead_api.c to make it a
> higher level aead api. The aes_ccm.c and aes_gcm.c are removed
> and all the functions are now implemented in their headers using
> the newly added aead api.
> 
No objection from me, though I'd ask you to respin with the indentation
fixed up a bit.

johannes

[toc] | [prev] | [next] | [standalone]

#1738231

2017-09-24 11:05 GMT-04:00 Johannes Berg <johannes@sipsolutions.net>:
> On Sun, 2017-09-24 at 01:40 -0400, Xiang Gao wrote:
>> Currently, the aes_ccm.c and aes_gcm.c are almost line by line
>> copy of each other. This patch reduce code redundancy by moving
>> the code in these two files to crypto/aead_api.c to make it a
>> higher level aead api. The aes_ccm.c and aes_gcm.c are removed
>> and all the functions are now implemented in their headers using
>> the newly added aead api.
>>
> No objection from me, though I'd ask you to respin with the indentation
> fixed up a bit.

Hi Johannes,

Thank you for you time for the suggestion. I'm not sure if I correctly
understand you point.
Do you mean to put more characters each line in the description? Something like:

> Currently, the aes_ccm.c and aes_gcm.c are almost line by line copy of
> each other. This patch reduce code redundancy by moving the code in these
> two files to crypto/aead_api.c to make it a higher level aead api. The
> file aes_ccm.c and aes_gcm.c are removed and all the functions there are
> now implemented in their headers using the newly added aead api.

instead of

> Currently, the aes_ccm.c and aes_gcm.c are almost line by line
> copy of each other. This patch reduce code redundancy by moving
> the code in these two files to crypto/aead_api.c to make it a
> higher level aead api. The aes_ccm.c and aes_gcm.c are removed
> and all the functions are now implemented in their headers using
> the newly added aead api.

Xiang Gao

>
> johannes

[toc] | [prev] | [next] | [standalone]

#1738237

On Sun, 2017-09-24 at 13:21 -0400, Xiang Gao wrote:
> 
> Do you mean to put more characters each line in the description
> 
Huh, sorry, no - my bad. I was thinking of the code, not the
description at all.

For example here:

> -int ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
> -                             u8 *data, size_t data_len, u8 *mic)
> +int aead_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, size_t aad_len,
> +                                u8 *data, size_t data_len, u8 *auth)
> 

I think you should adjust the indentation to match - or did it just get
mangled in my mail? It looks *further* indented now, when it should be
less (to after the opening parenthesis). Similarly in various other
places.

And perhaps for long things like

> +static inline struct crypto_aead *ieee80211_aes_key_setup_encrypt(
> +                               const u8 key[], size_t key_len,
> size_t mic_len)

> +struct crypto_aead *aead_key_setup_encrypt(const char *alg,
> +                       const u8 key[], size_t key_len, size_t authsize);

it might be better to write

static inline struct crypto_aead *
ieee80211_aes_key_setup_encrypt(const u8 key[], ...)

and

struct crypto_aead *
aead_key_setup_encrypt(const char *alg, ...)


respectively, depending on how far you have to indent to break lines
etc.

Anyway, I'm nitpicking.

Unrelated to this, I'm not sure whose tree this should go through -
probably Herbert's (or DaveM's with his ACK? not sure if there's a
crypto tree?) or so?

johannes

[toc] | [prev] | [next] | [standalone]

#1738249

2017-09-24 13:42 GMT-04:00 Johannes Berg <johannes@sipsolutions.net>:
> On Sun, 2017-09-24 at 13:21 -0400, Xiang Gao wrote:
>>
>> Do you mean to put more characters each line in the description
>>
> Huh, sorry, no - my bad. I was thinking of the code, not the
> description at all.

Oh yes, these indentation do looks ugly. Thank you for figuring this
out. The tab
width of my editor was set to 4. It should be 8... I will fix these
problems and resend
the patch soon, maybe after receiving a bit more feedback.

>
> For example here:
>
>> -int ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
>> -                             u8 *data, size_t data_len, u8 *mic)
>> +int aead_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, size_t aad_len,
>> +                                u8 *data, size_t data_len, u8 *auth)
>>
>
> I think you should adjust the indentation to match - or did it just get
> mangled in my mail? It looks *further* indented now, when it should be
> less (to after the opening parenthesis). Similarly in various other
> places.
>
> And perhaps for long things like
>
>> +static inline struct crypto_aead *ieee80211_aes_key_setup_encrypt(
>> +                               const u8 key[], size_t key_len,
>> size_t mic_len)
>
>> +struct crypto_aead *aead_key_setup_encrypt(const char *alg,
>> +                       const u8 key[], size_t key_len, size_t authsize);
>
> it might be better to write
>
> static inline struct crypto_aead *
> ieee80211_aes_key_setup_encrypt(const u8 key[], ...)
>
> and
>
> struct crypto_aead *
> aead_key_setup_encrypt(const char *alg, ...)
>
>
> respectively, depending on how far you have to indent to break lines
> etc.
>
> Anyway, I'm nitpicking.
>
> Unrelated to this, I'm not sure whose tree this should go through -
> probably Herbert's (or DaveM's with his ACK? not sure if there's a
> crypto tree?) or so?

Yes, there is one at
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git/
I'm not sure which tree to go either. I'm also not sure about the
beginning of the patch title,
should it be "mac80211:" or "crypto:"?

Options are:
1. This whole patch goes to either mac80211 tree or crypto tree. I
don't know which is better.
2. Make the higher level api only for internal usage in mac80211, i.e.
move the aead_api.c and aead_api.h to net/mac80211, does not export
the symbol. And of course, this will go to the mac80211 tree. I
personally don't want this to be the final solution because I happen
to be writing a loadable kernel module that uses these higher level
api.
3. Maybe split this patch, one for changes in crypto, which will go to
crypto tree, and the other for mac80211 part, which goes to the
mac80211 tree?

>
> johannes

[toc] | [prev] | [next] | [standalone]

#1738738

On Sun, Sep 24, 2017 at 07:42:46PM +0200, Johannes Berg wrote:
>
> Unrelated to this, I'm not sure whose tree this should go through -
> probably Herbert's (or DaveM's with his ACK? not sure if there's a
> crypto tree?) or so?

Since you're just rearranging code invoking the crypto API, rather
than touching actual crypto API code, I think you should handle it
as you do with any other wireless patch.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1738745

On Mon, 2017-09-25 at 12:56 +0800, Herbert Xu wrote:
> On Sun, Sep 24, 2017 at 07:42:46PM +0200, Johannes Berg wrote:
> > 
> > Unrelated to this, I'm not sure whose tree this should go through -
> > probably Herbert's (or DaveM's with his ACK? not sure if there's a
> > crypto tree?) or so?
> 
> Since you're just rearranging code invoking the crypto API, rather
> than touching actual crypto API code, I think you should handle it
> as you do with any other wireless patch.

The code moves to crypto/ though, and I'm not even sure I can vouch for
the Makefile choice there.

johannes

[toc] | [prev] | [next] | [standalone]

#1738758

On Mon, Sep 25, 2017 at 07:22:26AM +0200, Johannes Berg wrote:
> 
> The code moves to crypto/ though, and I'm not even sure I can vouch for
> the Makefile choice there.

Thanks, I missed that.  I don't think this belongs in crypto.
This proposed helper is only useful for wireless so it should
stay there.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web