Re: [PATCH v3 2/8] crypto: add driver-side scomp interface
On Fri, Mar 18, 2016 at 02:02:49PM +, Giovanni Cabiddu wrote: > > Are you suggesting to add to the struct acomp_req a pointer to a vmalloc > area and remove the request context (which is contiguous to acomp_req and > allocated in request_alloc)? What I'm suggesting is simply to generalise the noctx case so that it applies to acomp rather than just scomp. In fact, I think we should do it as a separate user interface, e.g., crypto_qdecomp that does the decompression without a vmalloc work space. You'd only have to implement decompression support for qdecomp since all compression algorithms require a vmalloc work space. So we would end up with crypto_acomp which always requires a vmalloc work space and crypto_qdecomp for the noctx case. Underneath you'd still maintain the two interfaces of acomp for the SG case and scomp for the linear case. > > Chances are that with hardware offload that you won't have to do > > such a vmalloc anyway. IOW acomp implementations are much more > > likely to be of the noctx type than scomp. > Our HW offload will always need a context area in the acomp_req but > I agree, there might be other acomp implementations which won't. That's interesting, why would your hardware need a vmalloc work space per-request? > Will allocate_space require a parameter which indicates the direction > of the operation (compression or decompression) or are we relying on the > user of the crypto API to know if he needs to call this function or not? At this point I don't believe the savings would be worth our effort so let's continue doing what we currently do which is to always allocate for both directions. Note that if you do the acomp/qdecomp split then you'd only do the allocation for acomp. In fact, we could then forget about the separate allocate_space call and just do it inside acomp_request_alloc as the noctx case would be accessed through the separate qdecomp interface. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/8] crypto: add driver-side scomp interface
Hi Herbert, I would like to have some clarifications before submitting a v4. On Thu, Mar 17, 2016 at 07:00:27PM +0800, Herbert Xu wrote: > On Wed, Feb 24, 2016 at 05:51:44PM +, Giovanni Cabiddu wrote: > > > > +#define CRYPTO_SCOMP_DECOMP_NOCTX CRYPTO_ALG_PRIVATE > > I don't think this should be an scomp-specific flag. The point of > noctx is that we don't need to do a request-specific vmalloc. Are you suggesting to add to the struct acomp_req a pointer to a vmalloc area and remove the request context (which is contiguous to acomp_req and allocated in request_alloc)? > Chances are that with hardware offload that you won't have to do > such a vmalloc anyway. IOW acomp implementations are much more > likely to be of the noctx type than scomp. Our HW offload will always need a context area in the acomp_req but I agree, there might be other acomp implementations which won't. > This is how I think it should work: > > 1. There should be a single request_alloc function that allocates > just the request without the vmalloc. > 2. You then have to call an alloc_space function to allocate the > vmalloc area (let's stop calling it ctx as it may lead to confusion > with other contexts in the crypto API). Will allocate_space require a parameter which indicates the direction of the operation (compression or decompression) or are we relying on the user of the crypto API to know if he needs to call this function or not? > The second step can obviously be skipped for the noctx case. > > Also instead of the private bit let's put this into the alg type: > > #define CRYPTO_ALG_TYPE_ACOMP 0x0008 > #define CRYPTO_ALG_TYPE_ACOMP_NOCTX0x0009 > #define CRYPTO_ALG_TYPE_SCOMP 0x000a > #define CRYPTO_ALG_TYPE_SCOMP_NOCTX0x000b The reason why Joonsoo Kim proposed the concept of NOCTX was to avoid allocating a decompression context. Are you suggesting also to extend this concept to compression as well? If yes we would need a way to indicate for which direction the driver requires the context/space. How can we specify that? > Now that we have killed crypto_hash we can shrink hash down to > two types and move it to 0xe, freeing up the space for acomp/scomp. Sounds good. Thank you, -- Giovanni -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/8] crypto: add driver-side scomp interface
On Wed, Feb 24, 2016 at 05:51:44PM +, Giovanni Cabiddu wrote: > > +#define CRYPTO_SCOMP_DECOMP_NOCTX CRYPTO_ALG_PRIVATE I don't think this should be an scomp-specific flag. The point of noctx is that we don't need to do a request-specific vmalloc. Chances are that with hardware offload that you won't have to do such a vmalloc anyway. IOW acomp implementations are much more likely to be of the noctx type than scomp. This is how I think it should work: 1. There should be a single request_alloc function that allocates just the request without the vmalloc. 2. You then have to call an alloc_space function to allocate the vmalloc area (let's stop calling it ctx as it may lead to confusion with other contexts in the crypto API). The second step can obviously be skipped for the noctx case. Also instead of the private bit let's put this into the alg type: #define CRYPTO_ALG_TYPE_ACOMP 0x0008 #define CRYPTO_ALG_TYPE_ACOMP_NOCTX0x0009 #define CRYPTO_ALG_TYPE_SCOMP 0x000a #define CRYPTO_ALG_TYPE_SCOMP_NOCTX0x000b Now that we have killed crypto_hash we can shrink hash down to two types and move it to 0xe, freeing up the space for acomp/scomp. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/8] crypto: add driver-side scomp interface
Add a synchronous back-end (scomp) to acomp. This allows to easily expose
the already present compression algorithms in LKCF via acomp.
Signed-off-by: Giovanni Cabiddu
---
crypto/Makefile |1 +
crypto/acompress.c | 75 ++-
crypto/scompress.c | 262 +++
include/crypto/acompress.h | 65 +++--
include/crypto/internal/acompress.h | 42 ++
include/crypto/internal/scompress.h | 138 ++
include/linux/crypto.h |9 +-
7 files changed, 543 insertions(+), 49 deletions(-)
create mode 100644 crypto/scompress.c
create mode 100644 include/crypto/internal/scompress.h
diff --git a/crypto/Makefile b/crypto/Makefile
index e817b38..fc8fcfe 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o
obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
obj-$(CONFIG_CRYPTO_ACOMP2) += acompress.o
+obj-$(CONFIG_CRYPTO_ACOMP2) += scompress.o
$(obj)/rsapubkey-asn1.o: $(obj)/rsapubkey-asn1.c $(obj)/rsapubkey-asn1.h
$(obj)/rsaprivkey-asn1.o: $(obj)/rsaprivkey-asn1.c $(obj)/rsaprivkey-asn1.h
diff --git a/crypto/acompress.c b/crypto/acompress.c
index f24fef3..2bd9c95 100644
--- a/crypto/acompress.c
+++ b/crypto/acompress.c
@@ -22,8 +22,11 @@
#include
#include
#include
+#include
#include "internal.h"
+static const struct crypto_type crypto_acomp_type;
+
#ifdef CONFIG_NET
static int crypto_acomp_report(struct sk_buff *skb, struct crypto_alg *alg)
{
@@ -67,6 +70,14 @@ static int crypto_acomp_init_tfm(struct crypto_tfm *tfm)
struct crypto_acomp *acomp = __crypto_acomp_tfm(tfm);
struct acomp_alg *alg = crypto_acomp_alg(acomp);
+ if (tfm->__crt_alg->cra_type != &crypto_acomp_type)
+ return crypto_init_scomp_ops_async(tfm);
+
+ acomp->compress = alg->compress;
+ acomp->decompress = alg->decompress;
+ acomp->comp_reqsize = alg->comp_reqsize;
+ acomp->decomp_reqsize = alg->decomp_reqsize;
+
if (alg->exit)
acomp->base.exit = crypto_acomp_exit_tfm;
@@ -76,15 +87,22 @@ static int crypto_acomp_init_tfm(struct crypto_tfm *tfm)
return 0;
}
+unsigned int crypto_acomp_extsize(struct crypto_alg *alg)
+{
+ if (alg->cra_type == &crypto_acomp_type)
+ return crypto_alg_extsize(alg);
+ return sizeof(struct crypto_scomp *);
+}
+
static const struct crypto_type crypto_acomp_type = {
- .extsize = crypto_alg_extsize,
+ .extsize = crypto_acomp_extsize,
.init_tfm = crypto_acomp_init_tfm,
#ifdef CONFIG_PROC_FS
.show = crypto_acomp_show,
#endif
.report = crypto_acomp_report,
.maskclear = ~CRYPTO_ALG_TYPE_MASK,
- .maskset = CRYPTO_ALG_TYPE_MASK,
+ .maskset = CRYPTO_ALG_TYPE_ACOMPRESS_MASK,
.type = CRYPTO_ALG_TYPE_ACOMPRESS,
.tfmsize = offsetof(struct crypto_acomp, base),
};
@@ -96,6 +114,59 @@ struct crypto_acomp *crypto_alloc_acomp(const char
*alg_name, u32 type,
}
EXPORT_SYMBOL_GPL(crypto_alloc_acomp);
+struct acomp_req *acomp_compression_request_alloc(struct crypto_acomp *acomp,
+ gfp_t gfp)
+{
+ struct crypto_tfm *tfm = crypto_acomp_tfm(acomp);
+ struct acomp_req *req;
+
+ req = __acomp_compression_request_alloc(acomp, gfp);
+ if (req && (tfm->__crt_alg->cra_type != &crypto_acomp_type))
+ return crypto_acomp_scomp_alloc_ctx(req, 1);
+
+ return req;
+}
+EXPORT_SYMBOL_GPL(acomp_compression_request_alloc);
+
+struct acomp_req *acomp_decompression_request_alloc(struct crypto_acomp *acomp,
+ gfp_t gfp)
+{
+ struct crypto_tfm *tfm = crypto_acomp_tfm(acomp);
+ struct acomp_req *req;
+
+ req = __acomp_decompression_request_alloc(acomp, gfp);
+ if (req && (tfm->__crt_alg->cra_type != &crypto_acomp_type))
+ return crypto_acomp_scomp_alloc_ctx(req, 0);
+
+ return req;
+}
+EXPORT_SYMBOL_GPL(acomp_decompression_request_alloc);
+
+struct acomp_req *acomp_request_alloc(struct crypto_acomp *acomp, gfp_t gfp)
+{
+ struct crypto_tfm *tfm = crypto_acomp_tfm(acomp);
+ struct acomp_req *req;
+
+ req = __acomp_request_alloc(acomp, gfp);
+ if (req && (tfm->__crt_alg->cra_type != &crypto_acomp_type))
+ return crypto_acomp_scomp_alloc_ctx(req, 1);
+
+ return req;
+}
+EXPORT_SYMBOL_GPL(acomp_request_alloc);
+
+void acomp_request_free(struct acomp_req *req)
+{
+ struct crypto_acomp *acomp = crypto_acomp_reqtfm(req);
+ struct crypto_tfm *tfm = crypto_acomp_tfm(acomp);
+
+ if (tfm->__crt_alg->cra_type != &crypto_acomp_type)
+ crypto_acomp_scomp_free_ctx(req);
+
+ __acomp_request_free(req);
+}
+EXPORT_SYMBOL_GPL(acomp_request_free);
+
int crypto_register_acomp(struct acomp_alg *alg)
{
struct cry
