lkml.org 
[lkml]   [2005]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 01/04] Adding cipher mode context information to crypto_tfm
    On Sat, 5 Feb 2005, Fruhwirth Clemens wrote:

    > Fixed formating and white-spaces. The biggest change is, that I stripped
    > a redundant check from scatterwalk.c. This omission seems justified and
    > shows no regression on my system. ( http://lkml.org/lkml/2005/2/3/87 )
    > Can you give it a quick test with ipsec anyhow? Just to make sure.

    Not tested yet, still reviewing the code.

    > + * The generic scatterwalker applies a certain function, pf, utilising
    > + * an arbitrary number of scatterlist data as it's arguments. These
    > + * arguments are supplied as an array of pointers to buffers. These
    > + * buffers are prepared and processed block-wise by the function
    > + * (stepsize) and might be input or output buffers.

    This is not going to work generically, as there number of atomic kmaps
    available is limited. You have four: one for input and one for output,
    each in user in softirq context (hence the list in crypto_km_types). A
    thread of execution can only use two at once (input & output). The crypto
    code is written around this: processing a cleartext and ciphertext block
    simultaneously.

    > +
    > +int scatterwalk_walk(sw_proc_func_t pf, void *priv, int steps,
    > + struct walk_info *walk_infos)
    > +{
    > + int r = -EINVAL;
    > +
    > + int i;

    Looks like this i is left over from something no longer use.

    > + scatterwalk_copychunks(*cbuf,&csg->sw,csg->stepsize,1);

    There are several places such as this where you use literal 1 & 0 instead
    of the new macros, SCATTERWALK_IO_OUTPUT & INPUT.

    > +static inline int scatterwalk_needscratch(struct scatter_walk *walk, int nbytes)
    > +{
    > + return (nbytes <= walk->len_this_page);
    > +}

    The logic of this is inverted given the function name. While the calling
    code is using it correctly, it's going to cause confusion.

    Also confusing is the remaining clamping of the page offset:

    static inline void scatterwalk_start(struct scatter_walk *walk, struct scatterlist *sg)
    {
    ...

    rest_of_page = PAGE_CACHE_SIZE - (sg->offset & (PAGE_CACHE_SIZE - 1));
    walk->len_this_page = min(sg->length, rest_of_page);
    }

    rest_of_page should be just PAGE_SIZE - sg->offset (sg->offset should
    never extend beyond the page).

    And then how would walk->len_this_page be greater than rest_of_page?


    - James
    --
    James Morris
    <jmorris@redhat.com>





    -
    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: 2005-03-22 14:10    [W:0.032 / U:0.036 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site