[PATCH 0/3] crypto: acomp - Add interface to set parameters

[PATCH 0/3] crypto: acomp - Add interface to set parameters

[Herbert Xu]

This patch series adds an interface to set compression algorithm
parameters.  The third patch is only an example.  Each algorithm
could come up with its own distinct set of parameters and format
if necessary.

Herbert Xu (3):
  crypto: scomp - Add setparam interface
  crypto: acomp - Add setparam interface
  crypto: acomp - Add comp_params helpers

 crypto/acompress.c                  |  70 +++++++++++++++++--
 crypto/compress.h                   |   9 ++-
 crypto/scompress.c                  | 103 +++++++++++++++++++++++++++-
 include/crypto/acompress.h          |  41 ++++++++++-
 include/crypto/internal/acompress.h |   3 +
 include/crypto/internal/scompress.h |  37 ++++++++++
 6 files changed, 252 insertions(+), 11 deletions(-)

-- 
2.39.2

[PATCH 1/3] crypto: scomp - Add setparam interface

[Herbert Xu]

Add the scompress plumbing for setparam.  This is modelled after
setkey for shash.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 crypto/scompress.c                  | 59 ++++++++++++++++++++++++++++-
 include/crypto/internal/scompress.h | 27 +++++++++++++
 2 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/crypto/scompress.c b/crypto/scompress.c
index 1cef6bb06a81..283fbea8336e 100644
--- a/crypto/scompress.c
+++ b/crypto/scompress.c
@@ -37,6 +37,51 @@ static const struct crypto_type crypto_scomp_type;
 static int scomp_scratch_users;
 static DEFINE_MUTEX(scomp_lock);
 
+static inline struct crypto_scomp *__crypto_scomp_cast(struct crypto_tfm *tfm)
+{
+	return container_of(tfm, struct crypto_scomp, base);
+}
+
+static int scomp_no_setparam(struct crypto_scomp *tfm, const u8 *param,
+			     unsigned int len)
+{
+	return -ENOSYS;
+}
+
+static bool crypto_scomp_alg_has_setparam(struct scomp_alg *alg)
+{
+	return alg->setparam != scomp_no_setparam;
+}
+
+static bool crypto_scomp_alg_needs_param(struct scomp_alg *alg)
+{
+	return crypto_scomp_alg_has_setparam(alg) &&
+	       !(alg->base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY);
+}
+
+static void scomp_set_need_param(struct crypto_scomp *tfm,
+				 struct scomp_alg *alg)
+{
+	if (crypto_scomp_alg_needs_param(alg))
+		crypto_scomp_set_flags(tfm, CRYPTO_TFM_NEED_KEY);
+}
+
+int crypto_scomp_setparam(struct crypto_scomp *tfm, const u8 *param,
+			  unsigned int len)
+{
+	struct scomp_alg *scomp = crypto_scomp_alg(tfm);
+	int err;
+
+	err = scomp->setparam(tfm, param, len);
+	if (unlikely(err)) {
+		scomp_set_need_param(tfm, scomp);
+		return err;
+	}
+
+	crypto_scomp_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
+	return 0;
+}
+
 static int __maybe_unused crypto_scomp_report(
 	struct sk_buff *skb, struct crypto_alg *alg)
 {
@@ -100,8 +145,12 @@ static int crypto_scomp_alloc_scratches(void)
 
 static int crypto_scomp_init_tfm(struct crypto_tfm *tfm)
 {
+	struct crypto_scomp *comp = __crypto_scomp_cast(tfm);
+	struct scomp_alg *alg = crypto_scomp_alg(comp);
 	int ret = 0;
 
+	scomp_set_need_param(comp, alg);
+
 	mutex_lock(&scomp_lock);
 	if (!scomp_scratch_users++)
 		ret = crypto_scomp_alloc_scratches();
@@ -277,11 +326,19 @@ static const struct crypto_type crypto_scomp_type = {
 	.tfmsize = offsetof(struct crypto_scomp, base),
 };
 
+static void scomp_prepare_alg(struct scomp_alg *alg)
+{
+	comp_prepare_alg(&alg->calg);
+
+	if (!alg->setparam)
+		alg->setparam = scomp_no_setparam;
+}
+
 int crypto_register_scomp(struct scomp_alg *alg)
 {
 	struct crypto_alg *base = &alg->calg.base;
 
-	comp_prepare_alg(&alg->calg);
+	scomp_prepare_alg(alg);
 
 	base->cra_type = &crypto_scomp_type;
 	base->cra_flags |= CRYPTO_ALG_TYPE_SCOMPRESS;
diff --git a/include/crypto/internal/scompress.h b/include/crypto/internal/scompress.h
index 07a10fd2d321..4a9cf2174c7a 100644
--- a/include/crypto/internal/scompress.h
+++ b/include/crypto/internal/scompress.h
@@ -27,6 +27,7 @@ struct crypto_scomp {
  * @free_ctx:	Function frees context allocated with alloc_ctx
  * @compress:	Function performs a compress operation
  * @decompress:	Function performs a de-compress operation
+ * @setparam:	Set parameters of the algorithm (e.g., compression level)
  * @base:	Common crypto API algorithm data structure
  * @calg:	Cmonn algorithm data structure shared with acomp
  */
@@ -39,6 +40,8 @@ struct scomp_alg {
 	int (*decompress)(struct crypto_scomp *tfm, const u8 *src,
 			  unsigned int slen, u8 *dst, unsigned int *dlen,
 			  void *ctx);
+	int (*setparam)(struct crypto_scomp *tfm, const u8 *param,
+			unsigned int len);
 
 	union {
 		struct COMP_ALG_COMMON;
@@ -71,6 +74,21 @@ static inline struct scomp_alg *crypto_scomp_alg(struct crypto_scomp *tfm)
 	return __crypto_scomp_alg(crypto_scomp_tfm(tfm)->__crt_alg);
 }
 
+static inline u32 crypto_scomp_get_flags(struct crypto_scomp *tfm)
+{
+	return crypto_tfm_get_flags(crypto_scomp_tfm(tfm));
+}
+
+static inline void crypto_scomp_set_flags(struct crypto_scomp *tfm, u32 flags)
+{
+	crypto_tfm_set_flags(crypto_scomp_tfm(tfm), flags);
+}
+
+static inline void crypto_scomp_clear_flags(struct crypto_scomp *tfm, u32 flags)
+{
+	crypto_tfm_clear_flags(crypto_scomp_tfm(tfm), flags);
+}
+
 static inline void *crypto_scomp_alloc_ctx(struct crypto_scomp *tfm)
 {
 	return crypto_scomp_alg(tfm)->alloc_ctx(tfm);
@@ -82,10 +100,16 @@ static inline void crypto_scomp_free_ctx(struct crypto_scomp *tfm,
 	return crypto_scomp_alg(tfm)->free_ctx(tfm, ctx);
 }
 
+int crypto_scomp_setparam(struct crypto_scomp *tfm, const u8 *param,
+			  unsigned int len);
+
 static inline int crypto_scomp_compress(struct crypto_scomp *tfm,
 					const u8 *src, unsigned int slen,
 					u8 *dst, unsigned int *dlen, void *ctx)
 {
+	if (crypto_scomp_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
+		return -ENOKEY;
+
 	return crypto_scomp_alg(tfm)->compress(tfm, src, slen, dst, dlen, ctx);
 }
 
@@ -94,6 +118,9 @@ static inline int crypto_scomp_decompress(struct crypto_scomp *tfm,
 					  u8 *dst, unsigned int *dlen,
 					  void *ctx)
 {
+	if (crypto_scomp_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
+		return -ENOKEY;
+
 	return crypto_scomp_alg(tfm)->decompress(tfm, src, slen, dst, dlen,
 						 ctx);
 }
-- 
2.39.2

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Sergey Senozhatsky]

On (24/05/20 19:04), Herbert Xu wrote:
[..]
> +int crypto_scomp_setparam(struct crypto_scomp *tfm, const u8 *param,
> +			  unsigned int len)
> +{
> +	struct scomp_alg *scomp = crypto_scomp_alg(tfm);
> +	int err;
> +
> +	err = scomp->setparam(tfm, param, len);
> +	if (unlikely(err)) {
> +		scomp_set_need_param(tfm, scomp);
> +		return err;
> +	}
> +
> +	crypto_scomp_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
> +	return 0;
> +}

Is the idea here that each compression driver will have its own structure
for params?

In other words, something like this?

static int setup_tfm(...)
{
...
	this_cpu->tfm = crypto_alloc_comp(name, 0, 0);

	if (!strcmp(name, "zstd")) {
		struct crypto_comp_param_zstd param;

		param.dict = ...
		param.cleve = ...

		crypto_scomp_setparam(tfm, &param, sizeof(param));
	}

	if (!strcmp(name, "lz4")) {
		struct crupto_comp_param_lz4 param;
		...
	}

	if (!strcmp(name, "lzo")) {
		struct crupto_comp_param_lzo param;
		...
	}
...
}

Or should it be "struct crypto_comp_params param"?

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Sergey Senozhatsky]

On (24/05/31 14:47), Sergey Senozhatsky wrote:
> On (24/05/20 19:04), Herbert Xu wrote:
> [..]
> > +int crypto_scomp_setparam(struct crypto_scomp *tfm, const u8 *param,
> > +			  unsigned int len)
> > +{
> > +	struct scomp_alg *scomp = crypto_scomp_alg(tfm);
> > +	int err;
> > +
> > +	err = scomp->setparam(tfm, param, len);
> > +	if (unlikely(err)) {
> > +		scomp_set_need_param(tfm, scomp);
> > +		return err;
> > +	}
> > +
> > +	crypto_scomp_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
> > +	return 0;
> > +}
> 
> Is the idea here that each compression driver will have its own structure
> for params?
> 
> In other words, something like this?
> 
> static int setup_tfm(...)
> {
> ...
> 	this_cpu->tfm = crypto_alloc_comp(name, 0, 0);
> 
> 	if (!strcmp(name, "zstd")) {
> 		struct crypto_comp_param_zstd param;
> 
> 		param.dict = ...
> 		param.cleve = ...
> 
> 		crypto_scomp_setparam(tfm, &param, sizeof(param));
> 	}
> 
> 	if (!strcmp(name, "lz4")) {
> 		struct crupto_comp_param_lz4 param;
> 		...
> 	}
> 
> 	if (!strcmp(name, "lzo")) {
> 		struct crupto_comp_param_lzo param;
> 		...
> 	}
> ...
> }
> 
> Or should it be "struct crypto_comp_params param"?

So passing "raw" algorithm parameters to crypto_scomp_setparam(tfm) can be
suboptimal, depending on the compression driver. For instance, for zstd
(what is currently done in zram [1]) we pre-process "raw" parameters:
parse dictionary in order to get zstd_cdict and zstd_ddict which are then
shared by all tfm-s (as they access C/D dictionaries in read-only mode).
For zram/zswap doing this per-tfm would result in extra per-CPU
zstd_cdict/zstd_ddict allocations, which is a significant overhead.

Does this sound like adding two more callbacks to drivers
(e.g. parseparam/freeparam)?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/block/zram/backend_zstd.c?h=next-20240529

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Herbert Xu]

On Fri, May 31, 2024 at 03:34:44PM +0900, Sergey Senozhatsky wrote:
>
> So passing "raw" algorithm parameters to crypto_scomp_setparam(tfm) can be
> suboptimal, depending on the compression driver. For instance, for zstd
> (what is currently done in zram [1]) we pre-process "raw" parameters:
> parse dictionary in order to get zstd_cdict and zstd_ddict which are then
> shared by all tfm-s (as they access C/D dictionaries in read-only mode).
> For zram/zswap doing this per-tfm would result in extra per-CPU
> zstd_cdict/zstd_ddict allocations, which is a significant overhead.

If they share the dictionary, why can't they just share the
tfm directly? Or do you actually need to vary the other parameters
while keeping the dictionary the same?

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

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Sergey Senozhatsky]

On (24/05/31 16:29), Herbert Xu wrote:
> On Fri, May 31, 2024 at 03:34:44PM +0900, Sergey Senozhatsky wrote:
> >
> > So passing "raw" algorithm parameters to crypto_scomp_setparam(tfm) can be
> > suboptimal, depending on the compression driver. For instance, for zstd
> > (what is currently done in zram [1]) we pre-process "raw" parameters:
> > parse dictionary in order to get zstd_cdict and zstd_ddict which are then
> > shared by all tfm-s (as they access C/D dictionaries in read-only mode).
> > For zram/zswap doing this per-tfm would result in extra per-CPU
> > zstd_cdict/zstd_ddict allocations, which is a significant overhead.
> 
> If they share the dictionary, why can't they just share the
> tfm directly? Or do you actually need to vary the other parameters
> while keeping the dictionary the same?

Is it possible to share a tfm? I thought that tfm-s carry some state
(compression workmem/scratch buffer) so one cannot do parallel compressions
on different CPUs using the same tfm.

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Herbert Xu]

On Sat, Jun 01, 2024 at 09:24:15AM +0900, Sergey Senozhatsky wrote:
>
> Is it possible to share a tfm? I thought that tfm-s carry some state
> (compression workmem/scratch buffer) so one cannot do parallel compressions
> on different CPUs using the same tfm.

Yes the tfm can be shared.  The data state is kept in the request
object.

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

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Sergey Senozhatsky]

On (24/06/01 11:54), Herbert Xu wrote:
> On Sat, Jun 01, 2024 at 09:24:15AM +0900, Sergey Senozhatsky wrote:
> >
> > Is it possible to share a tfm? I thought that tfm-s carry some state
> > (compression workmem/scratch buffer) so one cannot do parallel compressions
> > on different CPUs using the same tfm.
> 
> Yes the tfm can be shared.  The data state is kept in the request
> object.

Oh, nice, thanks.

I'll try that new API (and tfm sharing) in zram, coordinate with
android folks on this and will come back to you.

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Sergey Senozhatsky]

On (24/06/03 11:34), Sergey Senozhatsky wrote:
> On (24/06/01 11:54), Herbert Xu wrote:
> > On Sat, Jun 01, 2024 at 09:24:15AM +0900, Sergey Senozhatsky wrote:
> > >
> > > Is it possible to share a tfm? I thought that tfm-s carry some state
> > > (compression workmem/scratch buffer) so one cannot do parallel compressions
> > > on different CPUs using the same tfm.
> > 
> > Yes the tfm can be shared.  The data state is kept in the request
> > object.
> 
> Oh, nice, thanks.

Herbert, I'm not sure I see how tfm sharing is working.

crypto_tfm carries a pointer to __crt_ctx, which e.g. for zstd
is struct zstd_ctx, where it keeps all the state (zstd_cctx, zstd_dctx,
and compression/decompression workmem buffers).

When we call crypto_comp_compress()->zstd_compress() we just pass tfm,
then the driver gets tfm-s state/ctx via crypto_tfm_ctx() and uses it
for underlying compression library call.

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Herbert Xu]

On Mon, Jun 03, 2024 at 05:28:56PM +0900, Sergey Senozhatsky wrote:
>
> Herbert, I'm not sure I see how tfm sharing is working.
> 
> crypto_tfm carries a pointer to __crt_ctx, which e.g. for zstd
> is struct zstd_ctx, where it keeps all the state (zstd_cctx, zstd_dctx,
> and compression/decompression workmem buffers).

That's the legacy compression interface.  You should be looking
at the crypto_acomp interface which handles this properly.

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

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Sergey Senozhatsky]

On (24/06/03 16:34), Herbert Xu wrote:
> On Mon, Jun 03, 2024 at 05:28:56PM +0900, Sergey Senozhatsky wrote:
> >
> > Herbert, I'm not sure I see how tfm sharing is working.
> > 
> > crypto_tfm carries a pointer to __crt_ctx, which e.g. for zstd
> > is struct zstd_ctx, where it keeps all the state (zstd_cctx, zstd_dctx,
> > and compression/decompression workmem buffers).
> 
> That's the legacy compression interface.  You should be looking
> at the crypto_acomp interface which handles this properly.

Oh, I see, thanks, I didn't know about that, okay now I see what
you meant when you said that you'd not add setparams to legacy
scomp interface.

Alright, so this means

1) zram needs to be converted to acomp interface
2) scomp drivers that zram is using needs to become acomp compatible
   (for example, I don't think I see acomp support in crypto/zstd.c)
3) zram needs to support setparam
4) zram needs to support tfm sharing, so that setparam can be called
   once
5) crypto comp drivers need to start support setparam

That's quite a bit of work, I should admit.

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Herbert Xu]

On Tue, Jun 04, 2024 at 02:09:15PM +0900, Sergey Senozhatsky wrote:
> 
> Alright, so this means
> 
> 1) zram needs to be converted to acomp interface

Oops, somehow I was thinking of zswap rather than zram.

> 2) scomp drivers that zram is using needs to become acomp compatible
>    (for example, I don't think I see acomp support in crypto/zstd.c)

I think you're still confusing scomp with the legacy compress
interface that zram uses.  Any algorithm marked as scomp is using
the new acomp interface.  So zstd is fully available through acomp.

> 3) zram needs to support setparam
> 4) zram needs to support tfm sharing, so that setparam can be called
>    once
> 5) crypto comp drivers need to start support setparam
> 
> That's quite a bit of work, I should admit.

At least we won't be duplicating the compression algorithms.

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

Re: [PATCH 1/3] crypto: scomp - Add setparam interface

[Herbert Xu]

On Fri, May 31, 2024 at 02:47:59PM +0900, Sergey Senozhatsky wrote:
>
> Is the idea here that each compression driver will have its own structure
> for params?

The API is agnostic.  You can either have a common format shared
by all (your) algorithms, or you can do individual structures.

Since you're the only user, you get to make up the rules :)

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

[PATCH 2/3] crypto: acomp - Add setparam interface

[Herbert Xu]

Add the acompress plubming for setparam.  This is modelled after
setkey for ahash.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 crypto/acompress.c                  | 70 ++++++++++++++++++++++++++---
 crypto/compress.h                   |  9 +++-
 crypto/scompress.c                  |  9 +---
 include/crypto/acompress.h          | 32 ++++++++++++-
 include/crypto/internal/acompress.h |  3 ++
 5 files changed, 106 insertions(+), 17 deletions(-)

diff --git a/crypto/acompress.c b/crypto/acompress.c
index 6fdf0ff9f3c0..cf37243a2a3c 100644
--- a/crypto/acompress.c
+++ b/crypto/acompress.c
@@ -33,6 +33,55 @@ static inline struct acomp_alg *crypto_acomp_alg(struct crypto_acomp *tfm)
 	return __crypto_acomp_alg(crypto_acomp_tfm(tfm)->__crt_alg);
 }
 
+static int acomp_no_setparam(struct crypto_acomp *tfm, const u8 *param,
+			    unsigned int len)
+{
+	return -ENOSYS;
+}
+
+static int acomp_set_need_param(struct crypto_acomp *tfm,
+				 struct acomp_alg *alg)
+{
+	if (alg->calg.base.cra_type != &crypto_acomp_type) {
+		struct crypto_scomp **ctx = acomp_tfm_ctx(tfm);
+		struct crypto_scomp *scomp = *ctx;
+
+		if (!crypto_scomp_alg_has_setparam(crypto_scomp_alg(scomp)))
+			return 0;
+	} else if (alg->setparam == acomp_no_setparam)
+		return 0;
+
+	if ((alg->base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY))
+		crypto_acomp_set_flags(tfm, CRYPTO_TFM_NEED_KEY);
+
+	return 0;
+}
+
+int crypto_acomp_setparam(struct crypto_acomp *tfm, const u8 *param,
+			  unsigned int len)
+{
+	struct acomp_alg *alg = crypto_acomp_alg(tfm);
+	int err;
+
+	if (alg->calg.base.cra_type == &crypto_acomp_type)
+		err = alg->setparam(tfm, param, len);
+	else {
+		struct crypto_scomp **ctx = acomp_tfm_ctx(tfm);
+		struct crypto_scomp *scomp = *ctx;
+
+		err = crypto_scomp_setparam(scomp, param, len);
+	}
+
+	if (unlikely(err)) {
+		acomp_set_need_param(tfm, alg);
+		return err;
+	}
+
+	crypto_acomp_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(crypto_acomp_setparam);
+
 static int __maybe_unused crypto_acomp_report(
 	struct sk_buff *skb, struct crypto_alg *alg)
 {
@@ -66,8 +115,9 @@ 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);
+	if (alg->calg.base.cra_type != &crypto_acomp_type)
+		return crypto_init_scomp_ops_async(tfm) ?:
+		       acomp_set_need_param(acomp, alg);
 
 	acomp->compress = alg->compress;
 	acomp->decompress = alg->decompress;
@@ -77,10 +127,8 @@ static int crypto_acomp_init_tfm(struct crypto_tfm *tfm)
 	if (alg->exit)
 		acomp->base.exit = crypto_acomp_exit_tfm;
 
-	if (alg->init)
-		return alg->init(acomp);
-
-	return 0;
+	return (alg->init ? alg->init(acomp) : 0) ?:
+	       acomp_set_need_param(acomp, alg);
 }
 
 static unsigned int crypto_acomp_extsize(struct crypto_alg *alg)
@@ -160,11 +208,19 @@ void comp_prepare_alg(struct comp_alg_common *alg)
 	base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;
 }
 
+static void acomp_prepare_alg(struct acomp_alg *alg)
+{
+	comp_prepare_alg(&alg->calg);
+
+	if (!alg->setparam)
+		alg->setparam = acomp_no_setparam;
+}
+
 int crypto_register_acomp(struct acomp_alg *alg)
 {
 	struct crypto_alg *base = &alg->calg.base;
 
-	comp_prepare_alg(&alg->calg);
+	acomp_prepare_alg(alg);
 
 	base->cra_type = &crypto_acomp_type;
 	base->cra_flags |= CRYPTO_ALG_TYPE_ACOMPRESS;
diff --git a/crypto/compress.h b/crypto/compress.h
index c3cedfb5e606..aac8c24be688 100644
--- a/crypto/compress.h
+++ b/crypto/compress.h
@@ -9,15 +9,22 @@
 #ifndef _LOCAL_CRYPTO_COMPRESS_H
 #define _LOCAL_CRYPTO_COMPRESS_H
 
+#include <crypto/internal/scompress.h>
 #include "internal.h"
 
 struct acomp_req;
-struct comp_alg_common;
 
 int crypto_init_scomp_ops_async(struct crypto_tfm *tfm);
 struct acomp_req *crypto_acomp_scomp_alloc_ctx(struct acomp_req *req);
 void crypto_acomp_scomp_free_ctx(struct acomp_req *req);
+int scomp_no_setparam(struct crypto_scomp *tfm, const u8 *param,
+		      unsigned int len);
 
 void comp_prepare_alg(struct comp_alg_common *alg);
 
+static inline bool crypto_scomp_alg_has_setparam(struct scomp_alg *alg)
+{
+	return alg->setparam != scomp_no_setparam;
+}
+
 #endif	/* _LOCAL_CRYPTO_COMPRESS_H */
diff --git a/crypto/scompress.c b/crypto/scompress.c
index 283fbea8336e..9117d7c85f31 100644
--- a/crypto/scompress.c
+++ b/crypto/scompress.c
@@ -42,17 +42,12 @@ static inline struct crypto_scomp *__crypto_scomp_cast(struct crypto_tfm *tfm)
 	return container_of(tfm, struct crypto_scomp, base);
 }
 
-static int scomp_no_setparam(struct crypto_scomp *tfm, const u8 *param,
-			     unsigned int len)
+int scomp_no_setparam(struct crypto_scomp *tfm, const u8 *param,
+		      unsigned int len)
 {
 	return -ENOSYS;
 }
 
-static bool crypto_scomp_alg_has_setparam(struct scomp_alg *alg)
-{
-	return alg->setparam != scomp_no_setparam;
-}
-
 static bool crypto_scomp_alg_needs_param(struct scomp_alg *alg)
 {
 	return crypto_scomp_alg_has_setparam(alg) &&
diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
index 54937b615239..241d1dc5c883 100644
--- a/include/crypto/acompress.h
+++ b/include/crypto/acompress.h
@@ -125,6 +125,21 @@ static inline struct comp_alg_common *crypto_comp_alg_common(
 	return __crypto_comp_alg_common(crypto_acomp_tfm(tfm)->__crt_alg);
 }
 
+static inline u32 crypto_acomp_get_flags(struct crypto_acomp *tfm)
+{
+	return crypto_tfm_get_flags(crypto_acomp_tfm(tfm));
+}
+
+static inline void crypto_acomp_set_flags(struct crypto_acomp *tfm, u32 flags)
+{
+	crypto_tfm_set_flags(crypto_acomp_tfm(tfm), flags);
+}
+
+static inline void crypto_acomp_clear_flags(struct crypto_acomp *tfm, u32 flags)
+{
+	crypto_tfm_clear_flags(crypto_acomp_tfm(tfm), flags);
+}
+
 static inline unsigned int crypto_acomp_reqsize(struct crypto_acomp *tfm)
 {
 	return tfm->reqsize;
@@ -248,7 +263,12 @@ static inline void acomp_request_set_params(struct acomp_req *req,
  */
 static inline int crypto_acomp_compress(struct acomp_req *req)
 {
-	return crypto_acomp_reqtfm(req)->compress(req);
+	struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
+
+	if (crypto_acomp_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
+		return -ENOKEY;
+
+	return tfm->compress(req);
 }
 
 /**
@@ -262,7 +282,15 @@ static inline int crypto_acomp_compress(struct acomp_req *req)
  */
 static inline int crypto_acomp_decompress(struct acomp_req *req)
 {
-	return crypto_acomp_reqtfm(req)->decompress(req);
+	struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
+
+	if (crypto_acomp_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
+		return -ENOKEY;
+
+	return tfm->decompress(req);
 }
 
+int crypto_acomp_setparam(struct crypto_acomp *tfm,
+			  const u8 *param, unsigned int len);
+
 #endif
diff --git a/include/crypto/internal/acompress.h b/include/crypto/internal/acompress.h
index d00392d1988e..91c51526aac0 100644
--- a/include/crypto/internal/acompress.h
+++ b/include/crypto/internal/acompress.h
@@ -17,6 +17,7 @@
  *
  * @compress:	Function performs a compress operation
  * @decompress:	Function performs a de-compress operation
+ * @setparam:	Set parameters of the algorithm (e.g., compression level)
  * @dst_free:	Frees destination buffer if allocated inside the algorithm
  * @init:	Initialize the cryptographic transformation object.
  *		This function is used to initialize the cryptographic
@@ -37,6 +38,8 @@
 struct acomp_alg {
 	int (*compress)(struct acomp_req *req);
 	int (*decompress)(struct acomp_req *req);
+	int (*setparam)(struct crypto_acomp *tfm, const u8 *param,
+			unsigned int len);
 	void (*dst_free)(struct scatterlist *dst);
 	int (*init)(struct crypto_acomp *tfm);
 	void (*exit)(struct crypto_acomp *tfm);
-- 
2.39.2

Re: [PATCH 2/3] crypto: acomp - Add setparam interface

[Cabiddu, Giovanni]

Hi Herbert,

I'm resuming this as I would like to send an updated version of the
patch that converts BTRFS to use the acomp APIs [1].

On Mon, May 20, 2024 at 07:04:48PM +0800, Herbert Xu wrote:
> Add the acompress plubming for setparam.  This is modelled after
> setkey for ahash.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
>  crypto/acompress.c                  | 70 ++++++++++++++++++++++++++---
>  crypto/compress.h                   |  9 +++-
>  crypto/scompress.c                  |  9 +---
>  include/crypto/acompress.h          | 32 ++++++++++++-
>  include/crypto/internal/acompress.h |  3 ++
>  5 files changed, 106 insertions(+), 17 deletions(-)
> 
> diff --git a/crypto/acompress.c b/crypto/acompress.c
> index 6fdf0ff9f3c0..cf37243a2a3c 100644
> --- a/crypto/acompress.c
> +++ b/crypto/acompress.c
...
> +int crypto_acomp_setparam(struct crypto_acomp *tfm, const u8 *param,
> +			  unsigned int len)
Is the intent here to use strings to identify parameters? In such case,
`len` should be called `value`.
Or, is `param` a pointer to a structure?

In case `param` is a string, this would work ok with parameters that
just take a value, example of usage:

	tfm = crypto_alloc_acomp("deflate", 0, 0);
	crypto_acomp_setparam(tfm, COMP_ALG_COMPRESSION_LEVEL, 9);


Logic in algorithm:

#define COMP_ALG_COMPRESSION_LEVEL      "compression-level"
#define COMP_ALG_HISTORY_SIZE           "history-size"

enum {
	COMP_LEVEL,
	COMP_HISTORY_SIZE,
};

static const char * const param_type[] = {
	[COMP_LEVEL] = COMP_ALG_COMPRESSION_LEVEL,
	[COMP_HISTORY_SIZE] = COMP_ALG_HISTORY_SIZE,
};
static int deflate_setparam(struct crypto_acomp *tfm, const u8 *param,
			    unsigned int val)
{
	int ret;

	ret = sysfs_match_string(param_type, param);
	if (ret < 0)
		return ret;

	switch (ret) {
		case COMP_LEVEL:
			/* Set compression level */
			break;
		case COMP_HISTORY_SIZE:
			/* Set history size*/
			break;
		default:
			break;
	}

	return 0;
}

static struct acomp_alg acomp = {
	.compress		= deflate_compress,
	.decompress		= deflate_decompress,
	.setparam		= deflate_setparam,

Thanks,

-- 
Giovanni

[1] https://lore.kernel.org/all/20240426110941.5456-7-giovanni.cabiddu@intel.com/

Re: [PATCH 2/3] crypto: acomp - Add setparam interface

[Herbert Xu]

On Tue, May 06, 2025 at 05:01:27PM +0100, Cabiddu, Giovanni wrote:
>
> > diff --git a/crypto/acompress.c b/crypto/acompress.c
> > index 6fdf0ff9f3c0..cf37243a2a3c 100644
> > --- a/crypto/acompress.c
> > +++ b/crypto/acompress.c
> ...
> > +int crypto_acomp_setparam(struct crypto_acomp *tfm, const u8 *param,
> > +			  unsigned int len)
> Is the intent here to use strings to identify parameters? In such case,
> `len` should be called `value`.
> Or, is `param` a pointer to a structure?

param is just an arbitrary buffer with a length.  It's up to each
algorithm to put an interpretation on param.

But I would recommend going with the existing Crypto API norm of
using rtnl serialisation.

For example the existing struct zcomp_params (for zstd) would then
look like this under rtnl (copied from authenc):

	struct rtattr *rta = (struct rtattr *)param;
	struct crypto_zstd_param {
		__le32 dictlen;
		__le32 level;
	};

	struct crypto_zstd_param *zstd_param;

	if (!RTA_OK(rta, keylen))
		return -EINVAL;
	if (rta->rta_type != CRYPTO_AUTHENC_ZSTD_PARAM)
		return -EINVAL;

	if (RTA_PAYLOAD(rta) != sizeof(*param))
		return -EINVAL;

	zstd_param = RTA_DATA(rta);
	dictlen = le32_to_cpu(zstd_param->dictlen);
	level = le32_to_cpu(zstd_param->level);

	param += rta->rta_len;
	len -= rta->rta_len;

	if (len < dictlen)
		return -EINVAL;

	dict = param;

BTW Sergey said that he was going to work on this.  So you should
check in with him to see if he has any progress on this front.

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

Re: [PATCH 2/3] crypto: acomp - Add setparam interface

[Cabiddu, Giovanni]

On Wed, May 07, 2025 at 10:20:58AM +0800, Herbert Xu wrote:
> On Tue, May 06, 2025 at 05:01:27PM +0100, Cabiddu, Giovanni wrote:
> >
> > > diff --git a/crypto/acompress.c b/crypto/acompress.c
> > > index 6fdf0ff9f3c0..cf37243a2a3c 100644
> > > --- a/crypto/acompress.c
> > > +++ b/crypto/acompress.c
> > ...
> > > +int crypto_acomp_setparam(struct crypto_acomp *tfm, const u8 *param,
> > > +			  unsigned int len)
> > Is the intent here to use strings to identify parameters? In such case,
> > `len` should be called `value`.
> > Or, is `param` a pointer to a structure?
> 
> param is just an arbitrary buffer with a length.  It's up to each
> algorithm to put an interpretation on param.
> 
> But I would recommend going with the existing Crypto API norm of
> using rtnl serialisation.
> 
> For example the existing struct zcomp_params (for zstd) would then
> look like this under rtnl (copied from authenc):
> 
> 	struct rtattr *rta = (struct rtattr *)param;
> 	struct crypto_zstd_param {
> 		__le32 dictlen;
> 		__le32 level;
> 	};
> 
> 	struct crypto_zstd_param *zstd_param;
> 
> 	if (!RTA_OK(rta, keylen))
> 		return -EINVAL;
> 	if (rta->rta_type != CRYPTO_AUTHENC_ZSTD_PARAM)
> 		return -EINVAL;
> 
> 	if (RTA_PAYLOAD(rta) != sizeof(*param))
> 		return -EINVAL;
> 
> 	zstd_param = RTA_DATA(rta);
> 	dictlen = le32_to_cpu(zstd_param->dictlen);
> 	level = le32_to_cpu(zstd_param->level);
> 
> 	param += rta->rta_len;
> 	len -= rta->rta_len;
> 
> 	if (len < dictlen)
> 		return -EINVAL;
> 
> 	dict = param;
Thanks Herbert.

> BTW Sergey said that he was going to work on this.  So you should
> check in with him to see if he has any progress on this front.
Sergey, do you have an updated patchset?

If not, I can take over on this work. I have already rebased this version
against the latest head of cryptodev-2.6.

Regards,

-- 
Giovanni

Re: [PATCH 2/3] crypto: acomp - Add setparam interface

[Sergey Senozhatsky]

On (25/05/07 14:16), Cabiddu, Giovanni wrote:
[..]
> > param is just an arbitrary buffer with a length.  It's up to each
> > algorithm to put an interpretation on param.
> > 
> > But I would recommend going with the existing Crypto API norm of
> > using rtnl serialisation.
> > 
> > For example the existing struct zcomp_params (for zstd) would then
> > look like this under rtnl (copied from authenc):
> > 
> > 	struct rtattr *rta = (struct rtattr *)param;
> > 	struct crypto_zstd_param {
> > 		__le32 dictlen;
> > 		__le32 level;
> > 	};
> > 
> > 	struct crypto_zstd_param *zstd_param;
> > 
> > 	if (!RTA_OK(rta, keylen))
> > 		return -EINVAL;
> > 	if (rta->rta_type != CRYPTO_AUTHENC_ZSTD_PARAM)
> > 		return -EINVAL;
> > 
> > 	if (RTA_PAYLOAD(rta) != sizeof(*param))
> > 		return -EINVAL;
> > 
> > 	zstd_param = RTA_DATA(rta);
> > 	dictlen = le32_to_cpu(zstd_param->dictlen);
> > 	level = le32_to_cpu(zstd_param->level);
> > 
> > 	param += rta->rta_len;
> > 	len -= rta->rta_len;
> > 
> > 	if (len < dictlen)
> > 		return -EINVAL;
> > 
> > 	dict = param;
> Thanks Herbert.
> 
> > BTW Sergey said that he was going to work on this.  So you should
> > check in with him to see if he has any progress on this front.
> Sergey, do you have an updated patchset?

Hi,

I do have some in the making (a bit of a low priority recently),
not ready to show them yet.

> If not, I can take over on this work. I have already rebased this version
> against the latest head of cryptodev-2.6.

So these patches is step 0, there are a lot more steps after it.
I suppose you focus on IAA?

I'm in particular interested in lz4 and zstd (less so in deflate),
parameters support for those is currently implemented in "custom"
zram compression backends.

Re: [PATCH 2/3] crypto: acomp - Add setparam interface

[Eric Biggers]

On Wed, May 07, 2025 at 10:20:58AM +0800, Herbert Xu wrote:
> On Tue, May 06, 2025 at 05:01:27PM +0100, Cabiddu, Giovanni wrote:
> >
> > > diff --git a/crypto/acompress.c b/crypto/acompress.c
> > > index 6fdf0ff9f3c0..cf37243a2a3c 100644
> > > --- a/crypto/acompress.c
> > > +++ b/crypto/acompress.c
> > ...
> > > +int crypto_acomp_setparam(struct crypto_acomp *tfm, const u8 *param,
> > > +			  unsigned int len)
> > Is the intent here to use strings to identify parameters? In such case,
> > `len` should be called `value`.
> > Or, is `param` a pointer to a structure?
> 
> param is just an arbitrary buffer with a length.  It's up to each
> algorithm to put an interpretation on param.
> 
> But I would recommend going with the existing Crypto API norm of
> using rtnl serialisation.
> 
> For example the existing struct zcomp_params (for zstd) would then
> look like this under rtnl (copied from authenc):
> 
> 	struct rtattr *rta = (struct rtattr *)param;
> 	struct crypto_zstd_param {
> 		__le32 dictlen;
> 		__le32 level;
> 	};
> 
> 	struct crypto_zstd_param *zstd_param;
> 
> 	if (!RTA_OK(rta, keylen))
> 		return -EINVAL;
> 	if (rta->rta_type != CRYPTO_AUTHENC_ZSTD_PARAM)
> 		return -EINVAL;
> 
> 	if (RTA_PAYLOAD(rta) != sizeof(*param))
> 		return -EINVAL;
> 
> 	zstd_param = RTA_DATA(rta);
> 	dictlen = le32_to_cpu(zstd_param->dictlen);
> 	level = le32_to_cpu(zstd_param->level);
> 
> 	param += rta->rta_len;
> 	len -= rta->rta_len;
> 
> 	if (len < dictlen)
> 		return -EINVAL;
> 
> 	dict = param;

That sounds crazy.  There's no need to serialize and deserialize byte streams
just for different parts of the kernel to talk to each other.

I'm still skeptical about the whole idea of trying to force people to go through
the Crypto API for compression.  It just results in the adoption of all the bad
ideas from the Crypto API.

- Eric

Re: [PATCH 2/3] crypto: acomp - Add setparam interface

[Sergey Senozhatsky]

Cc-ing zswap and zram folks.

On (25/05/07 10:16), Eric Biggers wrote:
> On Wed, May 07, 2025 at 10:20:58AM +0800, Herbert Xu wrote:
> > On Tue, May 06, 2025 at 05:01:27PM +0100, Cabiddu, Giovanni wrote:
> > >
> > > > diff --git a/crypto/acompress.c b/crypto/acompress.c
> > > > index 6fdf0ff9f3c0..cf37243a2a3c 100644
> > > > --- a/crypto/acompress.c
> > > > +++ b/crypto/acompress.c
> > > ...
> > > > +int crypto_acomp_setparam(struct crypto_acomp *tfm, const u8 *param,
> > > > +			  unsigned int len)
> > > Is the intent here to use strings to identify parameters? In such case,
> > > `len` should be called `value`.
> > > Or, is `param` a pointer to a structure?
> > 
> > param is just an arbitrary buffer with a length.  It's up to each
> > algorithm to put an interpretation on param.
> > 
> > But I would recommend going with the existing Crypto API norm of
> > using rtnl serialisation.
> > 
> > For example the existing struct zcomp_params (for zstd) would then
> > look like this under rtnl (copied from authenc):
> > 
> > 	struct rtattr *rta = (struct rtattr *)param;
> > 	struct crypto_zstd_param {
> > 		__le32 dictlen;
> > 		__le32 level;
> > 	};
> > 
> > 	struct crypto_zstd_param *zstd_param;
> > 
> > 	if (!RTA_OK(rta, keylen))
> > 		return -EINVAL;
> > 	if (rta->rta_type != CRYPTO_AUTHENC_ZSTD_PARAM)
> > 		return -EINVAL;
> > 
> > 	if (RTA_PAYLOAD(rta) != sizeof(*param))
> > 		return -EINVAL;
> > 
> > 	zstd_param = RTA_DATA(rta);
> > 	dictlen = le32_to_cpu(zstd_param->dictlen);
> > 	level = le32_to_cpu(zstd_param->level);
> > 
> > 	param += rta->rta_len;
> > 	len -= rta->rta_len;
> > 
> > 	if (len < dictlen)
> > 		return -EINVAL;
> > 
> > 	dict = param;
> 
> That sounds crazy.  There's no need to serialize and deserialize byte streams
> just for different parts of the kernel to talk to each other.
> 
> I'm still skeptical about the whole idea of trying to force people to go through
> the Crypto API for compression.  It just results in the adoption of all the bad
> ideas from the Crypto API.

So in zram (for the time being) we use a very tiny (trivial) compression
API, which is more like wrappers around compression libs, we don't intend
to support anything other than that, sort of simplifies things a lot.  But
we are in a (slow) process of moving to async Crypto API.

I suspect there are compression modules in the kernel that have only Crypto
API hooks, e.g. drivers/crypto/intel/iaa/?  So in some cases (zswap + iaa)
I guess Crypto API is the only answer.

Re: [PATCH 2/3] crypto: acomp - Add setparam interface

[Kanchana P Sridhar]

Hi Sergey,

It is good to know that you are planning to move zram to the
async crypto API. As you correctly observed, the Intel IAA driver is
based on crypto acomp, which is Ok because we are currently working on
upstreaming “zswap compression batching” [1] as the first kernel user of
IAA. Our plan is to integrate IAA batching with zram, for which the
embedded crypto acomp dependencies in iaa_crypto are the first challenge
to overcome, while preserving the crypto interface for zswap. 

I would like to get your thoughts on whether the crypto_acomp batching
interface in patch 10 [2] of the above series would be acceptable for
zram IAA integration. If not, I was planning to create a layer in the
iaa_crypto driver that would allow interfacing with zcomp without
crypto. I figured it would be good to get some early feedback from you
before I work on a potential zcomp interface for iaa_crypto (which is
non-trivial), in light of your plans to move zram to use async crypto.

[1] https://patchwork.kernel.org/project/linux-mm/list/?series=958654
[2] https://patchwork.kernel.org/project/linux-mm/patch/20250430205305.22844-11-kanchana.p.sridhar@intel.com/

I will add you to [1].

Thanks,
Kanchana

[PATCH 3/3] crypto: acomp - Add comp_params helpers

[Herbert Xu]

Add helpers to get compression parameters, including the level
and an optional dictionary.

Note that algorithms do not have to use these helpers and could
come up with its own set of parameters.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
 crypto/scompress.c                  | 49 +++++++++++++++++++++++++++++
 include/crypto/acompress.h          |  9 ++++++
 include/crypto/internal/scompress.h | 10 ++++++
 3 files changed, 68 insertions(+)

diff --git a/crypto/scompress.c b/crypto/scompress.c
index 9117d7c85f31..1c35569df06c 100644
--- a/crypto/scompress.c
+++ b/crypto/scompress.c
@@ -14,6 +14,7 @@
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/rtnetlink.h>
 #include <linux/scatterlist.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
@@ -377,5 +378,53 @@ void crypto_unregister_scomps(struct scomp_alg *algs, int count)
 }
 EXPORT_SYMBOL_GPL(crypto_unregister_scomps);
 
+int crypto_comp_getparams(struct crypto_comp_params *params, const u8 *raw,
+			  unsigned int len)
+{
+	struct rtattr *rta = (struct rtattr *)raw;
+	void *dict;
+
+	crypto_comp_putparams(params);
+	params->level = CRYPTO_COMP_NO_LEVEL;
+
+	for (;; rta = RTA_NEXT(rta, len)) {
+		if (!RTA_OK(rta, len))
+			return -EINVAL;
+
+		if (rta->rta_type == CRYPTO_COMP_PARAM_LAST)
+			break;
+
+		switch (rta->rta_type) {
+		case CRYPTO_COMP_PARAM_LEVEL:
+			if (RTA_PAYLOAD(rta) != 4)
+				return -EINVAL;
+			memcpy(&params->level, RTA_DATA(rta), 4);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	dict = RTA_NEXT(rta, len);
+	if (!len)
+		return 0;
+
+	params->dict = kvmemdup(dict, len, GFP_KERNEL);
+	if (!params->dict)
+		return -ENOMEM;
+	params->dict_sz = len;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(crypto_comp_getparams);
+
+void crypto_comp_putparams(struct crypto_comp_params *params)
+{
+	kfree(params->dict);
+	params->dict = NULL;
+	params->dict_sz = 0;
+}
+EXPORT_SYMBOL_GPL(crypto_comp_putparams);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Synchronous compression type");
diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
index 241d1dc5c883..383a7f16b309 100644
--- a/include/crypto/acompress.h
+++ b/include/crypto/acompress.h
@@ -12,10 +12,19 @@
 #include <linux/atomic.h>
 #include <linux/container_of.h>
 #include <linux/crypto.h>
+#include <linux/limits.h>
 
 #define CRYPTO_ACOMP_ALLOC_OUTPUT	0x00000001
 #define CRYPTO_ACOMP_DST_MAX		131072
 
+#define CRYPTO_COMP_NO_LEVEL		INT_MIN
+
+enum {
+	CRYPTO_COMP_PARAM_UNSPEC,
+	CRYPTO_COMP_PARAM_LEVEL,
+	CRYPTO_COMP_PARAM_LAST,
+};
+
 /**
  * struct acomp_req - asynchronous (de)compression request
  *
diff --git a/include/crypto/internal/scompress.h b/include/crypto/internal/scompress.h
index 4a9cf2174c7a..995989a0c1ff 100644
--- a/include/crypto/internal/scompress.h
+++ b/include/crypto/internal/scompress.h
@@ -49,6 +49,12 @@ struct scomp_alg {
 	};
 };
 
+struct crypto_comp_params {
+	int level;
+	unsigned dict_sz;
+	void *dict;
+};
+
 static inline struct scomp_alg *__crypto_scomp_alg(struct crypto_alg *alg)
 {
 	return container_of(alg, struct scomp_alg, base);
@@ -150,4 +156,8 @@ void crypto_unregister_scomp(struct scomp_alg *alg);
 int crypto_register_scomps(struct scomp_alg *algs, int count);
 void crypto_unregister_scomps(struct scomp_alg *algs, int count);
 
+int crypto_comp_getparams(struct crypto_comp_params *params, const u8 *raw,
+			  unsigned int len);
+void crypto_comp_putparams(struct crypto_comp_params *params);
+
 #endif
-- 
2.39.2

Re: [PATCH 3/3] crypto: acomp - Add comp_params helpers

[Sergey Senozhatsky]

On (24/05/20 19:04), Herbert Xu wrote:
[..]
> +
> +	params->dict = kvmemdup(dict, len, GFP_KERNEL);
> +	if (!params->dict)
> +		return -ENOMEM;
> +	params->dict_sz = len;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(crypto_comp_getparams);
> +
> +void crypto_comp_putparams(struct crypto_comp_params *params)
> +{
> +	kfree(params->dict);

kvfree()?

> +	params->dict = NULL;
> +	params->dict_sz = 0;
> +}
> +EXPORT_SYMBOL_GPL(crypto_comp_putparams);

Re: [PATCH 3/3] crypto: acomp - Add comp_params helpers

[Herbert Xu]

On Fri, May 31, 2024 at 02:49:33PM +0900, Sergey Senozhatsky wrote:
> On (24/05/20 19:04), Herbert Xu wrote:
> [..]
> > +
> > +	params->dict = kvmemdup(dict, len, GFP_KERNEL);
> > +	if (!params->dict)
> > +		return -ENOMEM;
> > +	params->dict_sz = len;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(crypto_comp_getparams);
> > +
> > +void crypto_comp_putparams(struct crypto_comp_params *params)
> > +{
> > +	kfree(params->dict);
> 
> kvfree()?

Thanks.  I'll fix this in the next update.
-- 
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

Re: [PATCH 0/3] crypto: acomp - Add interface to set parameters

[Herbert Xu]

On Mon, May 20, 2024 at 07:04:43PM +0800, Herbert Xu wrote:
> This patch series adds an interface to set compression algorithm
> parameters.  The third patch is only an example.  Each algorithm
> could come up with its own distinct set of parameters and format
> if necessary.
> 
> Herbert Xu (3):
>   crypto: scomp - Add setparam interface
>   crypto: acomp - Add setparam interface
>   crypto: acomp - Add comp_params helpers
> 
>  crypto/acompress.c                  |  70 +++++++++++++++++--
>  crypto/compress.h                   |   9 ++-
>  crypto/scompress.c                  | 103 +++++++++++++++++++++++++++-
>  include/crypto/acompress.h          |  41 ++++++++++-
>  include/crypto/internal/acompress.h |   3 +
>  include/crypto/internal/scompress.h |  37 ++++++++++
>  6 files changed, 252 insertions(+), 11 deletions(-)
> 
> -- 
> 2.39.2

So does this satsify your needs Sergey? I'm not going to apply this
if you won't be using it.

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

Re: [PATCH 0/3] crypto: acomp - Add interface to set parameters

[Sergey Senozhatsky]

On (24/05/31 13:07), Herbert Xu wrote:
> On Mon, May 20, 2024 at 07:04:43PM +0800, Herbert Xu wrote:
> > This patch series adds an interface to set compression algorithm
> > parameters.  The third patch is only an example.  Each algorithm
> > could come up with its own distinct set of parameters and format
> > if necessary.
> > 
> > Herbert Xu (3):
> >   crypto: scomp - Add setparam interface
> >   crypto: acomp - Add setparam interface
> >   crypto: acomp - Add comp_params helpers
> > 
> >  crypto/acompress.c                  |  70 +++++++++++++++++--
> >  crypto/compress.h                   |   9 ++-
> >  crypto/scompress.c                  | 103 +++++++++++++++++++++++++++-
> >  include/crypto/acompress.h          |  41 ++++++++++-
> >  include/crypto/internal/acompress.h |   3 +
> >  include/crypto/internal/scompress.h |  37 ++++++++++
> >  6 files changed, 252 insertions(+), 11 deletions(-)
> > 
> > -- 
> > 2.39.2
> 
> So does this satsify your needs Sergey? I'm not going to apply this
> if you won't be using it.

Oh, I didn't see this series (not subscribed to linux-crypto).
Let me take a look.

Re: [PATCH 0/3] crypto: acomp - Add interface to set parameters

[Sergey Senozhatsky]

On (24/05/31 14:12), Sergey Senozhatsky wrote:
> On (24/05/31 13:07), Herbert Xu wrote:
> > On Mon, May 20, 2024 at 07:04:43PM +0800, Herbert Xu wrote:
> > > This patch series adds an interface to set compression algorithm
> > > parameters.  The third patch is only an example.  Each algorithm
> > > could come up with its own distinct set of parameters and format
> > > if necessary.
> > > 
> > > Herbert Xu (3):
> > >   crypto: scomp - Add setparam interface
> > >   crypto: acomp - Add setparam interface
> > >   crypto: acomp - Add comp_params helpers
> > > 
> > >  crypto/acompress.c                  |  70 +++++++++++++++++--
> > >  crypto/compress.h                   |   9 ++-
> > >  crypto/scompress.c                  | 103 +++++++++++++++++++++++++++-
> > >  include/crypto/acompress.h          |  41 ++++++++++-
> > >  include/crypto/internal/acompress.h |   3 +
> > >  include/crypto/internal/scompress.h |  37 ++++++++++
> > >  6 files changed, 252 insertions(+), 11 deletions(-)
> > > 
> > > -- 
> > > 2.39.2
> > 
> > So does this satsify your needs Sergey? I'm not going to apply this
> > if you won't be using it.
> 
> Oh, I didn't see this series (not subscribed to linux-crypto).
> Let me take a look.

Ah, wonderful. The series landed in the SPAM folder:

	"Why is this message in spam? It is similar to messages that were
	identified as spam in the past."

Whatever that means. Recovered, thanks for pinging me on this.