diff mbox

[v2,1/8] crypto: support (de)compression API that doesn't require tfm object

Message ID 1440052504-15442-2-git-send-email-iamjoonsoo.kim@lge.com (mailing list archive)
State Rejected
Delegated to: Herbert Xu
Headers show

Commit Message

Joonsoo Kim Aug. 20, 2015, 6:34 a.m. UTC
Until now, tfm object embeds (de)compression context in it and
(de)compression in crypto API requires tfm object to use
this context. But, there are some algorithms that doesn't need
such context to operate. Therefore, this patch introduce new crypto
compression API that call (de)compression function via crypto_alg,
instead of tfm object. crypto_alg is required to get appropriate
(de)compression function pointer. This can reduce overhead of
maintaining multiple tfm if (de)compression doesn't require
any context to operate.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 crypto/842.c           |  4 +++-
 crypto/compress.c      | 20 ++++++++++++++++++++
 crypto/crypto_null.c   |  4 +++-
 crypto/deflate.c       |  4 +++-
 crypto/lz4.c           |  4 +++-
 crypto/lz4hc.c         |  4 +++-
 crypto/lzo.c           |  4 +++-
 include/linux/crypto.h | 31 +++++++++++++++++++++++++++++++
 8 files changed, 69 insertions(+), 6 deletions(-)

Comments

Herbert Xu Aug. 20, 2015, 6:47 a.m. UTC | #1
On Thu, Aug 20, 2015 at 03:34:57PM +0900, Joonsoo Kim wrote:
> Until now, tfm object embeds (de)compression context in it and
> (de)compression in crypto API requires tfm object to use
> this context. But, there are some algorithms that doesn't need
> such context to operate. Therefore, this patch introduce new crypto
> compression API that call (de)compression function via crypto_alg,
> instead of tfm object. crypto_alg is required to get appropriate
> (de)compression function pointer. This can reduce overhead of
> maintaining multiple tfm if (de)compression doesn't require
> any context to operate.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

This isn't going to fly I'm afraid.  The main purpose of a tfm
is not to allocate memory but to track the crypto_alg object
which could be a hardware device.

So you're not going to get away with not allocating it.

What you can do for these contextless algorithms (and by definition
every compression algorithm is conxteless) is to allocate a system-
wide tfm that is used by everybody, or at least by every one within
your subsystem.

Cheers,
Herbert Xu Aug. 20, 2015, 7:50 a.m. UTC | #2
On Thu, Aug 20, 2015 at 04:52:17PM +0900, Joonsoo Kim wrote:
> 
> Hmm... I guess there is no problem. crypto_alg object fetched by
> crypto_get_comp() introduced in this patch could be hardware device
> algorithm which is same one that we can eventually fetch from tfm object.
> So, this approach would correctly track the crypto_alg regardless
> it is a hardware one or not. If there is some dependency between
> algorithm and tfm, it can't support _noctx API. Am I missing
> something?

Your approach limits what hardware devices we can support in
future.  It is fairly common for hardware drivers to only allocate
resources when a tfm is created.  With your tfmless interface,
the driver would have to unconditionally allocate resources.

It is essentially a global tfm without the tfm.

> Yes, I thought this way before, but, current way is much simpler so
> I try it first. If it is not acceptable, I will implement this
> approach.

Please go with a global tfm.

Thanks,
Joonsoo Kim Aug. 20, 2015, 7:52 a.m. UTC | #3
On Thu, Aug 20, 2015 at 02:47:28PM +0800, Herbert Xu wrote:
> On Thu, Aug 20, 2015 at 03:34:57PM +0900, Joonsoo Kim wrote:
> > Until now, tfm object embeds (de)compression context in it and
> > (de)compression in crypto API requires tfm object to use
> > this context. But, there are some algorithms that doesn't need
> > such context to operate. Therefore, this patch introduce new crypto
> > compression API that call (de)compression function via crypto_alg,
> > instead of tfm object. crypto_alg is required to get appropriate
> > (de)compression function pointer. This can reduce overhead of
> > maintaining multiple tfm if (de)compression doesn't require
> > any context to operate.
> > 
> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> This isn't going to fly I'm afraid.  The main purpose of a tfm
> is not to allocate memory but to track the crypto_alg object
> which could be a hardware device.
> 
> So you're not going to get away with not allocating it.

Hmm... I guess there is no problem. crypto_alg object fetched by
crypto_get_comp() introduced in this patch could be hardware device
algorithm which is same one that we can eventually fetch from tfm object.
So, this approach would correctly track the crypto_alg regardless
it is a hardware one or not. If there is some dependency between
algorithm and tfm, it can't support _noctx API. Am I missing
something?

> 
> What you can do for these contextless algorithms (and by definition
> every compression algorithm is conxteless) is to allocate a system-
> wide tfm that is used by everybody, or at least by every one within
> your subsystem.

Yes, I thought this way before, but, current way is much simpler so
I try it first. If it is not acceptable, I will implement this
approach.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joonsoo Kim Aug. 20, 2015, 8:05 a.m. UTC | #4
On Thu, Aug 20, 2015 at 03:50:35PM +0800, Herbert Xu wrote:
> On Thu, Aug 20, 2015 at 04:52:17PM +0900, Joonsoo Kim wrote:
> > 
> > Hmm... I guess there is no problem. crypto_alg object fetched by
> > crypto_get_comp() introduced in this patch could be hardware device
> > algorithm which is same one that we can eventually fetch from tfm object.
> > So, this approach would correctly track the crypto_alg regardless
> > it is a hardware one or not. If there is some dependency between
> > algorithm and tfm, it can't support _noctx API. Am I missing
> > something?
> 
> Your approach limits what hardware devices we can support in
> future.  It is fairly common for hardware drivers to only allocate
> resources when a tfm is created.  With your tfmless interface,
> the driver would have to unconditionally allocate resources.

Ah...Okay. thanks for clarifying.

> 
> It is essentially a global tfm without the tfm.
> 
> > Yes, I thought this way before, but, current way is much simpler so
> > I try it first. If it is not acceptable, I will implement this
> > approach.
> 
> Please go with a global tfm.

Okay. Will do that in next spin.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/crypto/842.c b/crypto/842.c
index 98e387e..b6503ea 100644
--- a/crypto/842.c
+++ b/crypto/842.c
@@ -61,7 +61,9 @@  static struct crypto_alg alg = {
 	.cra_module		= THIS_MODULE,
 	.cra_u			= { .compress = {
 	.coa_compress		= crypto842_compress,
-	.coa_decompress		= crypto842_decompress } }
+	.coa_decompress		= crypto842_decompress,
+	.coa_compress_noctx	= NULL,
+	.coa_decompress_noctx	= NULL } }
 };
 
 static int __init crypto842_mod_init(void)
diff --git a/crypto/compress.c b/crypto/compress.c
index c33f076..f27eee0 100644
--- a/crypto/compress.c
+++ b/crypto/compress.c
@@ -33,6 +33,26 @@  static int crypto_decompress(struct crypto_tfm *tfm,
 	                                                   dlen);
 }
 
+struct crypto_alg *crypto_get_comp(const char *alg_name, u32 type, u32 mask)
+{
+	struct crypto_alg *alg;
+
+	type &= ~CRYPTO_ALG_TYPE_MASK;
+	type |= CRYPTO_ALG_TYPE_COMPRESS;
+	mask |= CRYPTO_ALG_TYPE_MASK;
+
+	alg = crypto_alg_mod_lookup(alg_name, type, mask);
+	if (!IS_ERR(alg))
+		return alg;
+
+	return NULL;
+}
+
+void crypto_put_comp(struct crypto_alg *alg)
+{
+	crypto_mod_put(alg);
+}
+
 int crypto_init_compress_ops(struct crypto_tfm *tfm)
 {
 	struct compress_tfm *ops = &tfm->crt_compress;
diff --git a/crypto/crypto_null.c b/crypto/crypto_null.c
index 941c9a4..e397d1c 100644
--- a/crypto/crypto_null.c
+++ b/crypto/crypto_null.c
@@ -146,7 +146,9 @@  static struct crypto_alg null_algs[3] = { {
 	.cra_module		=	THIS_MODULE,
 	.cra_u			=	{ .compress = {
 	.coa_compress		=	null_compress,
-	.coa_decompress		=	null_compress } }
+	.coa_decompress		=	null_compress,
+	.coa_compress_noctx	=	NULL,
+	.coa_decompress_noctx	=	NULL } }
 } };
 
 MODULE_ALIAS_CRYPTO("compress_null");
diff --git a/crypto/deflate.c b/crypto/deflate.c
index 95d8d37..dee4daf 100644
--- a/crypto/deflate.c
+++ b/crypto/deflate.c
@@ -203,7 +203,9 @@  static struct crypto_alg alg = {
 	.cra_exit		= deflate_exit,
 	.cra_u			= { .compress = {
 	.coa_compress 		= deflate_compress,
-	.coa_decompress  	= deflate_decompress } }
+	.coa_decompress		= deflate_decompress,
+	.coa_compress_noctx	= NULL,
+	.coa_decompress_noctx	= NULL } }
 };
 
 static int __init deflate_mod_init(void)
diff --git a/crypto/lz4.c b/crypto/lz4.c
index aefbcea..1848435 100644
--- a/crypto/lz4.c
+++ b/crypto/lz4.c
@@ -86,7 +86,9 @@  static struct crypto_alg alg_lz4 = {
 	.cra_exit		= lz4_exit,
 	.cra_u			= { .compress = {
 	.coa_compress		= lz4_compress_crypto,
-	.coa_decompress		= lz4_decompress_crypto } }
+	.coa_decompress		= lz4_decompress_crypto,
+	.coa_compress_noctx	= NULL,
+	.coa_decompress_noctx	= NULL } }
 };
 
 static int __init lz4_mod_init(void)
diff --git a/crypto/lz4hc.c b/crypto/lz4hc.c
index a1d3b5b..bcf0baa 100644
--- a/crypto/lz4hc.c
+++ b/crypto/lz4hc.c
@@ -86,7 +86,9 @@  static struct crypto_alg alg_lz4hc = {
 	.cra_exit		= lz4hc_exit,
 	.cra_u			= { .compress = {
 	.coa_compress		= lz4hc_compress_crypto,
-	.coa_decompress		= lz4hc_decompress_crypto } }
+	.coa_decompress		= lz4hc_decompress_crypto,
+	.coa_compress_noctx	= NULL,
+	.coa_decompress_noctx	= NULL } }
 };
 
 static int __init lz4hc_mod_init(void)
diff --git a/crypto/lzo.c b/crypto/lzo.c
index 4b3e925..9ca516b 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -89,7 +89,9 @@  static struct crypto_alg alg = {
 	.cra_exit		= lzo_exit,
 	.cra_u			= { .compress = {
 	.coa_compress 		= lzo_compress,
-	.coa_decompress  	= lzo_decompress } }
+	.coa_decompress		= lzo_decompress,
+	.coa_compress_noctx	= NULL,
+	.coa_decompress_noctx	= NULL } }
 };
 
 static int __init lzo_mod_init(void)
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 81ef938..9e68eb0 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -405,6 +405,10 @@  struct compress_alg {
 			    unsigned int slen, u8 *dst, unsigned int *dlen);
 	int (*coa_decompress)(struct crypto_tfm *tfm, const u8 *src,
 			      unsigned int slen, u8 *dst, unsigned int *dlen);
+	int (*coa_compress_noctx)(const u8 *src, unsigned int slen,
+				  u8 *dst, unsigned int *dlen);
+	int (*coa_decompress_noctx)(const u8 *src, unsigned int slen,
+				    u8 *dst, unsigned int *dlen);
 };
 
 
@@ -1888,6 +1892,9 @@  static inline void crypto_free_comp(struct crypto_comp *tfm)
 	crypto_free_tfm(crypto_comp_tfm(tfm));
 }
 
+struct crypto_alg *crypto_get_comp(const char *alg_name, u32 type, u32 mask);
+void crypto_put_comp(struct crypto_alg *alg);
+
 static inline int crypto_has_comp(const char *alg_name, u32 type, u32 mask)
 {
 	type &= ~CRYPTO_ALG_TYPE_MASK;
@@ -1923,5 +1930,29 @@  static inline int crypto_comp_decompress(struct crypto_comp *tfm,
 						    src, slen, dst, dlen);
 }
 
+static inline int crypto_has_compress_noctx(struct crypto_alg *alg)
+{
+	return !!alg->cra_compress.coa_compress_noctx;
+}
+
+static inline int crypto_has_decompress_noctx(struct crypto_alg *alg)
+{
+	return !!alg->cra_compress.coa_decompress_noctx;
+}
+
+static inline int crypto_comp_compress_noctx(struct crypto_alg *alg,
+					const u8 *src, unsigned int slen,
+					u8 *dst, unsigned int *dlen)
+{
+	return alg->cra_compress.coa_compress_noctx(src, slen, dst, dlen);
+}
+
+static inline int crypto_comp_decompress_noctx(struct crypto_alg *alg,
+					const u8 *src, unsigned int slen,
+					u8 *dst, unsigned int *dlen)
+{
+	return alg->cra_compress.coa_decompress_noctx(src, slen, dst, dlen);
+}
+
 #endif	/* _LINUX_CRYPTO_H */