Initial implementation of acomp-dpusm provider #2

gcabiddu · 2025-05-07T16:16:30Z

Add an initial implementation of the acomp-dpusm provider.

This requires the following components:

Linux kernel - versions from v6.4 to v6.6 as this component requires acomp(zlib-deflate).
DPUSM: https://github.com/hpc/dpusm
ZFS with Z.I.A: https://github.com/hpc/zfs/commits/zia/

Add initial version of gitignore file. Signed-off-by: Giovanni Cabiddu <[email protected]>

gcabiddu · 2025-05-07T16:25:58Z

Adding @calccrypto to the review.

Copilot

Pull Request Overview

This PR introduces the initial implementation of the acomp-dpusm provider to integrate DPUSM with acomp (zlib-deflate) support. Key changes include:

Adding provider.c with memory management, compress/decompress wrappers, and provider registration.
Creating compress.h and compress.c to define and implement the acomp compression/decompression interface with scatterlist support.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File	Description
provider.c	Implements the provider functions, including buffer handling and registration routines.
compress.h	Declares the interface for acomp compression and decompression functions.
compress.c	Provides the implementation for accom compression/decompression using crypto and scatterlists.

Files not reviewed (1)

Makefile: Language not supported

Comments suppressed due to low confidence (1)

provider.c:19

[nitpick] The prefix 'ZQH' used in the enum values is not immediately descriptive; consider adding a clarifying comment or using more intuitive names to improve code readability.

enum handle_type { ZQH_REAL, ZQH_REF, };

Copilot · 2025-05-07T20:24:45Z

provider.c

+	if (ret)
+		return DPUSM_ERROR;
+
+	/*


Clarify and handle the overflow case properly in acomp_compress rather than returning a generic error; consider implementing logic to handle -EOVERFLOW to ensure data integrity.

Add initial implementation of acomp-dpusm plug-in. Signed-off-by: Giovanni Cabiddu <[email protected]>

Copilot

Pull Request Overview

Adds the initial implementation of the ACOMP DPUSM provider, including kernel integration, compression logic, and documentation.

Introduce provider.c as the DPUSM provider interface for ZFS ACOMP
Add compress.c/compress.h implementing zlib-deflate operations via the Crypto API
Provide README.md and INSTALL.md for usage and installation instructions

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
provider.c	Implements DPUSM provider functions and lifecycle
compress.h	Declares acomp API for compression/decompression
compress.c	Implements scatterlist-based zlib-deflate operations
README.md	Project overview, requirements, and limitations
INSTALL.md	Step-by-step installation and configuration guide

Files not reviewed (1)

Makefile: Language not supported

Comments suppressed due to low confidence (1)

provider.c:152

[nitpick] Consider adding unit or integration tests to validate compression/decompression behavior, including overflow and error paths.

static int zfs_acomp_compress(dpusm_compress_t alg, int level, void *src,

Copilot · 2025-05-13T21:07:14Z

provider.c

+		return NULL;
+
+	ref->type = ZQH_REF;
+	ref->ptr = ptr_start(src, offset);


Missing boundary check: ensure that offset + size does not exceed the source buffer size before creating a reference handle.

Copilot · 2025-05-13T21:07:14Z

provider.c

+	if (ret)
+		return DPUSM_ERROR;
+
+	/*
+	 * TODO: understand what to do in case of overflow. In such case, ret
+	 * is equals to -EOVERFLOW
+	 */
+


Unaddressed overflow case: handle the scenario where acomp_compress returns -EOVERFLOW to avoid silent failures or data corruption.

Suggested change

if (ret)

return DPUSM_ERROR;

/*

* TODO: understand what to do in case of overflow. In such case, ret

* is equals to -EOVERFLOW

*/

if (ret == -EOVERFLOW) {

/*

* Handle the overflow case by returning a specific error code.

* This allows the caller to take appropriate action.

*/

return DPUSM_OVERFLOW;

} else if (ret) {

return DPUSM_ERROR;

}

Copilot · 2025-05-13T21:07:15Z

compress.h

+int acomp_compress(void *src, int src_len, void *dst, int dst_len, size_t *c_len);
+int acomp_decompress(void *src, int src_len, void *dst, int dst_len, size_t *c_len);


API signature uses int for buffer lengths, which may truncate large size_t values; consider switching src_len and dst_len to size_t.

Suggested change

int acomp_compress(void *src, int src_len, void *dst, int dst_len, size_t *c_len);

int acomp_decompress(void *src, int src_len, void *dst, int dst_len, size_t *c_len);

int acomp_compress(void *src, size_t src_len, void *dst, size_t dst_len, size_t *c_len);

int acomp_decompress(void *src, size_t src_len, void *dst, size_t dst_len, size_t *c_len);

Copilot · 2025-05-13T21:07:15Z

README.md

+
+## Software Requirements
+
+- Linux Kernel with support for the Crypto API's acomp interface and zlib-deflate (currently covering versions 6.3 to 6.6).


Documentation lists kernel versions 6.3–6.6, but the PR description specifies 6.4–6.6; update the version range for consistency.

Suggested change

- Linux Kernel with support for the Crypto API's acomp interface and zlib-deflate (currently covering versions 6.3 to 6.6).

- Linux Kernel with support for the Crypto API's acomp interface and zlib-deflate (currently covering versions 6.4 to 6.6).

Copilot · 2025-05-13T21:07:15Z

INSTALL.md

+
+## Runtime Configuration
+
+Ensure the zpools are configired to use the provider and enable compression/decompression.  The operations would look like:


configired is misspelled; should be configured.

Suggested change

Ensure the zpools are configired to use the provider and enable compression/decompression. The operations would look like:

Ensure the zpools are configured to use the provider and enable compression/decompression. The operations would look like:

calccrypto · 2025-05-14T13:30:05Z

INSTALL.md

+
+### Obtain ZFS with ZIA
+
+To use ZFS with ZFS Integrated Acceleration (ZIA), you need to obtain the specific pull request (PR) that includes ZIA support. Follow these steps:


ZFS Interface for Accelerators (Z.I.A.)

Thanks @calccrypto - resolved.

calccrypto · 2025-05-14T13:30:33Z

README.md

+
+## Introduction
+
+The Acompress Provider is a plugin designed to integrate with DPUSM (Data Processing Unit Storage Management) and ZFS, utilizing the Z.I.A (ZFS Integrated Acceleration) framework. It enhances data compression capabilities within ZFS environments by leveraging the Crypto API's acomp interface for efficient data processing and storage management.


ZFS Interface for Accelerators (Z.I.A.)

Thanks @calccrypto - resolved.

Add intial contents for README and INSTALL documents. Signed-off-by: Joel Schuetze <[email protected]>

acomp-dpusm: add gitignore

a4c90dd

Add initial version of gitignore file. Signed-off-by: Giovanni Cabiddu <[email protected]>

gcabiddu requested review from Copilot and jdschuet May 7, 2025 16:16

This comment was marked as resolved.

Sign in to view

gcabiddu force-pushed the devel branch from 647c4fc to 41bd3d3 Compare May 7, 2025 16:24

gcabiddu requested a review from Copilot May 7, 2025 16:53

This comment was marked as resolved.

Sign in to view

gcabiddu requested a review from Copilot May 7, 2025 19:00

This comment was marked as resolved.

Sign in to view

gcabiddu force-pushed the devel branch from 41bd3d3 to adef3d2 Compare May 7, 2025 20:21

gcabiddu requested a review from Copilot May 7, 2025 20:24

Copilot AI reviewed May 7, 2025

View reviewed changes

gcabiddu force-pushed the devel branch from adef3d2 to a4fc0da Compare May 13, 2025 10:56

acomp-dpusm: add initial implementation

f9d99f7

Add initial implementation of acomp-dpusm plug-in. Signed-off-by: Giovanni Cabiddu <[email protected]>

gcabiddu force-pushed the devel branch from a4fc0da to f9d99f7 Compare May 13, 2025 14:57

gcabiddu requested a review from Copilot May 13, 2025 21:04

Copilot AI reviewed May 13, 2025

View reviewed changes

gcabiddu force-pushed the devel branch from aa87316 to 928caab Compare May 13, 2025 21:13

calccrypto reviewed May 14, 2025

View reviewed changes

acomp-dpusm: add README and INSTALL

fe53b8d

Add intial contents for README and INSTALL documents. Signed-off-by: Joel Schuetze <[email protected]>

gcabiddu force-pushed the devel branch from 928caab to fe53b8d Compare May 14, 2025 15:18

		int acomp_compress(void src, int src_len, void dst, int dst_len, size_t *c_len);
		int acomp_decompress(void src, int src_len, void dst, int dst_len, size_t *c_len);


		## Software Requirements

		- Linux Kernel with support for the Crypto API's acomp interface and zlib-deflate (currently covering versions 6.3 to 6.6).


		## Runtime Configuration

		Ensure the zpools are configired to use the provider and enable compression/decompression. The operations would look like:

	Ensure the zpools are configired to use the provider and enable compression/decompression. The operations would look like:
	Ensure the zpools are configured to use the provider and enable compression/decompression. The operations would look like:


		### Obtain ZFS with ZIA

		To use ZFS with ZFS Integrated Acceleration (ZIA), you need to obtain the specific pull request (PR) that includes ZIA support. Follow these steps:


		## Introduction

		The Acompress Provider is a plugin designed to integrate with DPUSM (Data Processing Unit Storage Management) and ZFS, utilizing the Z.I.A (ZFS Integrated Acceleration) framework. It enhances data compression capabilities within ZFS environments by leveraging the Crypto API's acomp interface for efficient data processing and storage management.

Initial implementation of acomp-dpusm provider #2

Are you sure you want to change the base?

Initial implementation of acomp-dpusm provider #2

Uh oh!

Conversation

gcabiddu commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

gcabiddu commented May 7, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 13, 2025

Choose a reason for hiding this comment

Uh oh!

calccrypto May 14, 2025

Choose a reason for hiding this comment

Uh oh!

gcabiddu May 14, 2025

Choose a reason for hiding this comment

Uh oh!

calccrypto May 14, 2025

Choose a reason for hiding this comment

Uh oh!

gcabiddu May 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gcabiddu commented May 7, 2025 •

edited

Loading