lkml.org 
[lkml]   [2009]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] crypto: compress - Add pcomp interface
    	Hi Herbert,

    On Wed, 14 Jan 2009, Herbert Xu wrote:
    > On Tue, Jan 13, 2009 at 04:59:40PM +0100, Geert Uytterhoeven wrote:
    > > The current "comp" crypto interface supports one-shot (de)compression only,
    > > i.e. the whole data buffer to be (de)compressed must be passed at once, and
    > > the whole (de)compressed data buffer will be received at once.
    > > In several use-cases (e.g. compressed file systems that store files in big
    > > compressed blocks), this workflow is not suitable.
    > > Furthermore, the "comp" type doesn't provide for the configuration of
    > > (de)compression parameters, and always allocates workspace memory for both
    > > compression and decompression, which may waste memory.
    >
    > Thanks for the patch-set!
    >
    > > diff --git a/crypto/pcompress.c b/crypto/pcompress.c
    > > new file mode 100644
    > > index 0000000..b9e3724
    > > --- /dev/null
    > > +++ b/crypto/pcompress.c
    > > @@ -0,0 +1,94 @@
    > > +/*
    > > + * Cryptographic API.
    > > + *
    > > + * Partial compression operations.
    > > + *
    > > + * Copyright 2008 Sony Corporation
    > > + *
    > > + * This program is free software; you can redistribute it and/or modify
    > > + * it under the terms of the GNU General Public License as published by
    > > + * the Free Software Foundation; version 2 of the License.
    > > + *
    > > + * This program is distributed in the hope that it will be useful,
    > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
    > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
    > > + * GNU General Public License for more details.
    > > + *
    > > + * You should have received a copy of the GNU General Public License
    > > + * along with this program; if not, write to the Free Software Foundation,
    > > + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
    > > + */
    > > +
    > > +#define pr_fmt(fmt) "%s: " fmt, __func__
    >
    > This appears to be unused?

    It's used by the pr_*() macros in <linux/kernel.h>.

    Since commit d091c2f58ba32029495a933b721e8e02fbd12caa ("Add 'pr_fmt()' format
    modifier to pr_xyz macros."), this is the new way to have a common prefix in
    all printed output.

    > > +static int crypto_pcomp_init_tfm(struct crypto_tfm *tfm,
    > > + const struct crypto_type *frontend)
    > > +{
    > > + if (frontend->type != CRYPTO_ALG_TYPE_PCOMPRESS)
    > > + return -EINVAL;
    >
    > This is my fault. The check is redundant so you can remove it.
    > I'll kill it in shash too.

    Ok, removed.

    > > +struct pcomp_alg {
    > > + int (*setup)(struct crypto_tfm *tfm, const void *params);
    >
    > Actually I was thinking of separate alloc functions for compress
    > and decompress so that

    For compatibility with crypto_comp, we also need an alloc function for both
    compress and decompress, so that makes 3 alloc functions.

    That would also solve the issue where there's hardware support for e.g.
    decompression, but not for compression.

    > 1) We know what the user wants to do without every algorithm
    > reinventing their own signalling for it;

    I guess you want to use the flags to indicate compress/decompress/both?
    Unfortunately I'm still struggling to fully understand the type/mask handling,
    so I would appreciate it if you could give me a hint how to handle that.

    > 2) The parameters can be separated.

    OK.

    > Also, this is something that we'll potentially export to user-space,
    > so we need to ensure that it is invariant to word length.
    >
    > So something like this would be good
    >
    > int (*setup_comp)(struct crypto_pcomp *tfm, const void *params,
    > unsigned int length);
    > int (*setup_decomp)(struct crypto_pcomp *tfm, const void *params,
    > unsigned int length);

    I assume "length" is the size of the passed params, so the algorithms can
    return -EINVAL if they're passed the wrong size?

    > The actual parameters should be formatted using the netlink helpers
    > (nla_*). So each parameter that you want to set should show up as
    > a netlink attribute. If a paramter is absent then you'd just use
    > the default, etc.

    I'll look into the netlink formatting...

    > > + int (*compress_init)(struct crypto_tfm *tfm);
    >
    > That should be struct crypto_pcomp.

    OK, I updated all pcomp_alg operations to take crypto_pcomp pointers instead of
    crypto_tfm pointers.

    > > +static inline struct crypto_pcomp *crypto_alloc_pcomp(const char *alg_name,
    > > + u32 type, u32 mask)
    > > +{
    > > + type &= ~CRYPTO_ALG_TYPE_MASK;
    > > + type |= CRYPTO_ALG_TYPE_PCOMPRESS;
    > > + mask |= CRYPTO_ALG_TYPE_MASK;
    > > +
    > > + return __crypto_pcomp_cast(crypto_alloc_base(alg_name, type, mask));
    > > +}
    >
    > That's the old way to allocate tfm's which won't work since pcomp
    > is using the new type API. You should do it the way that shash
    > does it.

    I tried to do this, but stumbled across a dependency problem: as
    crypto_alloc_tfm() needs a pointer to crypto_pcomp_type(), crypto_alloc_pcomp()
    can no longer be static inline, and must be moved to crypto/pcompress.c.

    However, pcompress can be a module, while testmgr (which uses
    crypto_alloc_pcomp()) seems to be always builtin.

    Ah, CRYPTO_MANAGER2 has to select CRYPTO_PCOMP. OK.

    Thanks for your review!

    With kind regards,

    Geert Uytterhoeven
    Software Architect

    Sony Techsoft Centre Europe
    The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

    Phone: +32 (0)2 700 8453
    Fax: +32 (0)2 700 8622
    E-mail: Geert.Uytterhoeven@sonycom.com
    Internet: http://www.sony-europe.com/

    A division of Sony Europe (Belgium) N.V.
    VAT BE 0413.825.160 · RPR Brussels
    Fortis · BIC GEBABEBB · IBAN BE41293037680010
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2009-01-14 16:05    [W:0.031 / U:29.544 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site