diff mbox series

[v1,3/3] zram: introduce crypto-api backend

Message ID 20241119122713.3294173-4-avromanov@salutedevices.com (mailing list archive)
State New
Headers show
Series zram: introduce crypto-backend api | expand

Commit Message

Alexey Romanov Nov. 19, 2024, 12:27 p.m. UTC
Since we use custom backend implementation, we remove the ability
for users to use algorithms from crypto backend. This breaks
backward compatibility, user doesn't necessarily use one of the
algorithms from "custom" backends defined in zram folder.
For example, he can use some driver with hardware compression support.

This patch adds opinion to enable Crypto API: add ZRAM_BACKEND_CRYPTO_API.
Option is enabled by default, because in previously version of ZRAM
it was possible to choose any alogirthm using Crypto API. This is
also done for backward compatibility purposes.

Signed-off-by: Alexey Romanov <avromanov@salutedevices.com>
---
 drivers/block/zram/Kconfig              |  10 ++
 drivers/block/zram/Makefile             |   1 +
 drivers/block/zram/backend_crypto_api.c | 117 ++++++++++++++++++++++++
 drivers/block/zram/backend_crypto_api.h |  10 ++
 drivers/block/zram/zcomp.c              |  73 +++++++++++++--
 5 files changed, 203 insertions(+), 8 deletions(-)
 create mode 100644 drivers/block/zram/backend_crypto_api.c
 create mode 100644 drivers/block/zram/backend_crypto_api.h

Comments

Christoph Hellwig Nov. 19, 2024, 12:34 p.m. UTC | #1
On Tue, Nov 19, 2024 at 03:27:13PM +0300, Alexey Romanov wrote:
> Since we use custom backend implementation, we remove the ability
> for users to use algorithms from crypto backend. This breaks
> backward compatibility, user doesn't necessarily use one of the
> algorithms from "custom" backends defined in zram folder.
> For example, he can use some driver with hardware compression support.
> 
> This patch adds opinion to enable Crypto API: add ZRAM_BACKEND_CRYPTO_API.
> Option is enabled by default, because in previously version of ZRAM
> it was possible to choose any alogirthm using Crypto API. This is
> also done for backward compatibility purposes.

Which crypto API algorithm do you care about?  You should probably
just add a backend for that instead of a double indirection.
Alexey Romanov Nov. 19, 2024, 1:04 p.m. UTC | #2
Hi Christoph,

On Tue, Nov 19, 2024 at 04:34:52AM -0800, Christoph Hellwig wrote:
> On Tue, Nov 19, 2024 at 03:27:13PM +0300, Alexey Romanov wrote:
> > Since we use custom backend implementation, we remove the ability
> > for users to use algorithms from crypto backend. This breaks
> > backward compatibility, user doesn't necessarily use one of the
> > algorithms from "custom" backends defined in zram folder.
> > For example, he can use some driver with hardware compression support.
> > 
> > This patch adds opinion to enable Crypto API: add ZRAM_BACKEND_CRYPTO_API.
> > Option is enabled by default, because in previously version of ZRAM
> > it was possible to choose any alogirthm using Crypto API. This is
> > also done for backward compatibility purposes.
> 
> Which crypto API algorithm do you care about?  You should probably
> just add a backend for that instead of a double indirection.
> 

Should I create backend_*.c file for every compression algo driver?
Okay, there aren't many of them now. But what will do, for example,
when there will be 250 such compression drivers?

And also your approach doesn't allow working with loadable modules.
For example, we may have only binary module (without sources) from
vendor SDK that provieds a driver for data compression. 

This is an even bigger problem.
Christoph Hellwig Nov. 19, 2024, 1:08 p.m. UTC | #3
On Tue, Nov 19, 2024 at 01:04:44PM +0000, Alexey Romanov wrote:
> Should I create backend_*.c file for every compression algo driver?

No.

> Okay, there aren't many of them now. But what will do, for example,
> when there will be 250 such compression drivers?

Why would there?

> 
> And also your approach doesn't allow working with loadable modules.
> For example, we may have only binary module (without sources) from
> vendor SDK that provieds a driver for data compression. 

Well, maybe just piss off instead of sneaking in hooks for your
illegal binary modules.
kernel test robot Nov. 22, 2024, 2:59 p.m. UTC | #4
Hi Alexey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v6.12 next-20241122]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexey-Romanov/zram-pass-zcomp-instead-of-zcomp_params-to-create_context-method/20241121-135511
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20241119122713.3294173-4-avromanov%40salutedevices.com
patch subject: [PATCH v1 3/3] zram: introduce crypto-api backend
config: x86_64-randconfig-012-20241122 (https://download.01.org/0day-ci/archive/20241122/202411222217.SkVD8y1f-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241122/202411222217.SkVD8y1f-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411222217.SkVD8y1f-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/block/zram/zcomp.c:233:12: warning: 'init_crypto_api_backends' defined but not used [-Wunused-function]
     233 | static int init_crypto_api_backends(void)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/init_crypto_api_backends +233 drivers/block/zram/zcomp.c

   232	
 > 233	static int init_crypto_api_backends(void)
   234	{
   235		struct crypto_alg *alg;
   236		struct zcomp_ops *ops;
   237	
   238		list_for_each_entry(alg, &crypto_alg_list, cra_list) {
   239			if (!crypto_has_comp(alg->cra_name, 0, 0))
   240				continue;
   241	
   242			if (backend_enabled(alg->cra_name))
   243				continue;
   244	
   245			ops = get_backend_crypto_api(alg->cra_name);
   246			if (IS_ERR_OR_NULL(ops))
   247				return PTR_ERR(ops);
   248	
   249			list_add(&ops->list, &backends);
   250		}
   251	
   252		return 0;
   253	}
   254
diff mbox series

Patch

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 6aea609b795c..672578040912 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -57,6 +57,16 @@  config ZRAM_BACKEND_LZO
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 
+config ZRAM_BACKEND_CRYPTO_API
+	bool "Enable support for compression algorithms available in Crypto API"
+	depends on ZRAM
+	default y
+	select CRYPTO
+	help
+	  If you still want to use Crypto API as a backend, enable this option.
+	  All compression algorithms enabled on your system will be available in ZRAM.
+	  This option is useful if you are using hardware compression using any driver.
+
 choice
 	prompt "Default zram compressor"
 	default ZRAM_DEF_COMP_LZORLE
diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
index 0fdefd576691..d12d03a01f35 100644
--- a/drivers/block/zram/Makefile
+++ b/drivers/block/zram/Makefile
@@ -8,5 +8,6 @@  zram-$(CONFIG_ZRAM_BACKEND_LZ4HC)	+= backend_lz4hc.o
 zram-$(CONFIG_ZRAM_BACKEND_ZSTD)	+= backend_zstd.o
 zram-$(CONFIG_ZRAM_BACKEND_DEFLATE)	+= backend_deflate.o
 zram-$(CONFIG_ZRAM_BACKEND_842)		+= backend_842.o
+zram-$(CONFIG_ZRAM_BACKEND_CRYPTO_API)	+= backend_crypto_api.o
 
 obj-$(CONFIG_ZRAM)	+=	zram.o
diff --git a/drivers/block/zram/backend_crypto_api.c b/drivers/block/zram/backend_crypto_api.c
new file mode 100644
index 000000000000..1cd792b77aac
--- /dev/null
+++ b/drivers/block/zram/backend_crypto_api.c
@@ -0,0 +1,117 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/crypto.h>
+
+#include "backend_crypto_api.h"
+
+struct crypto_api_ctx {
+	struct crypto_comp *tfm;
+};
+
+extern struct list_head crypto_alg_list;
+
+static void crypto_api_release_params(struct zcomp_params *params)
+{
+}
+
+static int crypto_api_setup_params(struct zcomp_params *params)
+{
+	return 0;
+}
+
+static int crypto_api_create(struct zcomp *zcomp, struct zcomp_ctx *ctx)
+{
+	struct crypto_api_ctx *crypto_ctx;
+	const char *algname = zcomp->ops->name;
+
+	crypto_ctx = kzalloc(sizeof(*crypto_ctx), GFP_KERNEL);
+	if (!crypto_ctx)
+		return -ENOMEM;
+
+	crypto_ctx->tfm = crypto_alloc_comp(algname, 0, 0);
+	if (IS_ERR_OR_NULL(crypto_ctx->tfm)) {
+		kfree(crypto_ctx);
+		return -ENOMEM;
+	}
+
+	ctx->context = crypto_ctx;
+
+	return 0;
+}
+
+static void crypto_api_destroy(struct zcomp_ctx *ctx)
+{
+	struct crypto_api_ctx *crypto_ctx = ctx->context;
+
+	if (!IS_ERR_OR_NULL(crypto_ctx->tfm))
+		crypto_free_comp(crypto_ctx->tfm);
+
+	kfree(crypto_ctx);
+}
+
+static int crypto_api_compress(struct zcomp_params *params, struct zcomp_ctx *ctx,
+			       struct zcomp_req *req)
+{
+	struct crypto_api_ctx *crypto_ctx = ctx->context;
+	unsigned int dst_len = req->dst_len;
+	int ret;
+
+	ret = crypto_comp_compress(crypto_ctx->tfm,
+				   req->src, req->src_len,
+				   req->dst, &dst_len);
+
+	req->dst_len = dst_len;
+
+	return ret;
+}
+
+static int crypto_api_decompress(struct zcomp_params *params, struct zcomp_ctx *ctx,
+				 struct zcomp_req *req)
+{
+	struct crypto_api_ctx *crypto_ctx = ctx->context;
+	unsigned int dst_len = req->dst_len;
+	int ret;
+
+	ret = crypto_comp_decompress(crypto_ctx->tfm,
+				     req->src, req->src_len,
+				     req->dst, &dst_len);
+
+	req->dst_len = dst_len;
+
+	return ret;
+}
+
+static void crypto_api_destroy_ops(struct zcomp_ops *ops)
+{
+	kfree(ops->name);
+	kfree(ops);
+}
+
+struct zcomp_ops *get_backend_crypto_api(const char *name)
+{
+	struct zcomp_ops *ops;
+	char *algname;
+
+	ops = kmalloc(sizeof(*ops), GFP_KERNEL);
+	if (!ops)
+		return ERR_PTR(-ENOMEM);
+
+	algname = kstrdup(name, GFP_KERNEL);
+	if (!algname) {
+		kfree(ops);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ops->compress = crypto_api_compress;
+	ops->decompress = crypto_api_decompress,
+	ops->create_ctx = crypto_api_create,
+	ops->destroy_ctx = crypto_api_destroy,
+	ops->setup_params = crypto_api_setup_params,
+	ops->release_params = crypto_api_release_params,
+	ops->destroy = crypto_api_destroy_ops,
+	ops->name = algname;
+
+	return ops;
+}
diff --git a/drivers/block/zram/backend_crypto_api.h b/drivers/block/zram/backend_crypto_api.h
new file mode 100644
index 000000000000..5ff8f75efdb4
--- /dev/null
+++ b/drivers/block/zram/backend_crypto_api.h
@@ -0,0 +1,10 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#ifndef __BACKEND_CRYPTO_API_H__
+#define __BACKEND_CRYPTO_API_H__
+
+#include "zcomp.h"
+
+struct zcomp_ops *get_backend_crypto_api(const char *name);
+
+#endif /* __BACKEND_CRYPTO_API_H__ */
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 40b5ab4c598b..da4b370c31c1 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -20,6 +20,9 @@ 
 #include "backend_zstd.h"
 #include "backend_deflate.h"
 #include "backend_842.h"
+#include "backend_crypto_api.h"
+
+extern struct list_head crypto_alg_list;
 
 static LIST_HEAD(backends);
 
@@ -216,63 +219,117 @@  void clean_zcomp_backends(void)
 		backend->destroy(backend);
 }
 
+static bool backend_enabled(const char *name)
+{
+	struct zcomp_ops *backend;
+
+	list_for_each_entry(backend, &backends, list)
+		if (!strcmp(backend->name, name))
+			return true;
+
+	return false;
+}
+
+static int init_crypto_api_backends(void)
+{
+	struct crypto_alg *alg;
+	struct zcomp_ops *ops;
+
+	list_for_each_entry(alg, &crypto_alg_list, cra_list) {
+		if (!crypto_has_comp(alg->cra_name, 0, 0))
+			continue;
+
+		if (backend_enabled(alg->cra_name))
+			continue;
+
+		ops = get_backend_crypto_api(alg->cra_name);
+		if (IS_ERR_OR_NULL(ops))
+			return PTR_ERR(ops);
+
+		list_add(&ops->list, &backends);
+	}
+
+	return 0;
+}
+
 int init_zcomp_backends(void)
 {
 	struct zcomp_ops *ops;
+	int ret;
 
 #if IS_ENABLED(CONFIG_ZRAM_BACKEND_LZO)
 	ops = get_backend_lzorle();
-	if (IS_ERR_OR_NULL(ops))
+	if (IS_ERR_OR_NULL(ops)) {
+		ret = PTR_ERR(ops);
 		goto err;
+	}
 
 	list_add(&ops->list, &backends);
 
 	ops = get_backend_lzo();
-	if (IS_ERR_OR_NULL(ops))
+	if (IS_ERR_OR_NULL(ops)) {
+		ret = PTR_ERR(ops);
 		goto err;
+	}
 
 	list_add(&ops->list, &backends);
 #endif
 #if IS_ENABLED(CONFIG_ZRAM_BACKEND_LZ4)
 	ops = get_backend_lz4();
-	if (IS_ERR_OR_NULL(ops))
+	if (IS_ERR_OR_NULL(ops)) {
+		ret = PTR_ERR(ops);
 		goto err;
+	}
 
 	list_add(&ops->list, &backends);
 #endif
 #if IS_ENABLED(CONFIG_ZRAM_BACKEND_LZ4HC)
 	ops = get_backend_lz4hc();
-	if (IS_ERR_OR_NULL(ops))
+	if (IS_ERR_OR_NULL(ops)) {
+		ret = PTR_ERR(ops);
 		goto err;
+	}
 
 	list_add(&ops->list, &backends);
 #endif
 #if IS_ENABLED(CONFIG_ZRAM_BACKEND_ZSTD)
 	ops = get_backend_zstd();
-	if (IS_ERR_OR_NULL(ops))
+	if (IS_ERR_OR_NULL(ops)) {
+		ret = PTR_ERR(ops);
 		goto err;
+	}
 
 	list_add(&ops->list, &backends);
 #endif
 #if IS_ENABLED(CONFIG_ZRAM_BACKEND_DEFLATE)
 	ops = get_backend_deflate();
-	if (IS_ERR_OR_NULL(ops))
+	if (IS_ERR_OR_NULL(ops)) {
+		ret = PTR_ERR(ops);
 		goto err;
+	}
 
 	list_add(&ops->list, &backends);
 #endif
 #if IS_ENABLED(CONFIG_ZRAM_BACKEND_842)
 	ops = get_backend_842();
-	if (IS_ERR_OR_NULL(ops))
+	if (IS_ERR_OR_NULL(ops)) {
+		ret = PTR_ERR(ops);
 		goto err;
+	}
 
 	list_add(&ops->list, &backends);
 #endif
 
+#if IS_ENABLED(CONFIG_ZRAM_BACKEND_CRYPTO_API)
+	ret = init_crypto_api_backends();
+	if (ret)
+		goto err;
+#endif
+
 	return 0;
 
 err:
 	clean_zcomp_backends();
 
-	return PTR_ERR(ops);
+	return ret;
 }