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.
