Hi.
On Fri, Oct 24, 2008 at 01:54:23PM -0700, Shasi Pulijala ([EMAIL PROTECTED])
wrote:
> This patch v7 includes a few coding style changes and benchmark comparison
> between for tcrypt and cryptodev. These are just a rough estimate and not the
> exact numbers for cbc-aes. CryptoDev interface will always be slight more as
> it includes crypto setup and a few other housekeeping tasks. Tcrypt is just
> the time for the encrypt or decrypt function calls only.
>
> Tcrypt Cryptodev
> 16 byte 47us 132us
> 32 byte 51us 141us
> 64 byte 68us 150us
Can you also run cryptodev under profiler to determine, why it is, well,
noticebly, slower than tcrypt?
> With regard to forward declaration, they are needed.
Why? Can't you reorder functions to eliminate that?
> >> + }
> >> + sg_set_page(&sg[x], page, bufsize, offset);
> >> + rop = PAGE_SIZE - sg[x].offset;
> >> + if (bufsize > rop) {
> >> + sg[x].length = rop;
> >> + bufsize = bufsize - rop;
> >> + }
> >> + offset = 0;
> >>
> >>What if bufsize is smaller than 'rop', but there are several pages?
> >>This will initialize wrong buffers, but probably there is a check somewhere
> >>above this layer.
>
> Yes, there is a check above for the number of pages returned. It returns an
> error if the number of pages returned is greater/less than requested.
>
> With regard to "Still this is not an async crypto," it is asynchronous
> interface if called via AIO. For vector write, it will turn into synchronous
> AIO within the kernel AIO framework.
It does not mean it is async, since crypto processing itself is
synchronous. I.e. you can submit requests asynchronously, but they are
processed by the crypto code one-by-one with waiting after each request.
This actually can explain your numbers: instead of having flow of
requests, you have to do really lots of work per-request. I do not say
this is wrong (although imho it should be done differently), since it is
your code and likely it fits your needs.
Plus, you did not figure what happens when request completion is
interrupted with regard to freeing data, which may be accesed by the
crypto code in parallel.
> + aead_request_set_crypt(req, ssg, dsg, bufsize, cop->iv);
> + aead_request_set_assoc(req, &adata, cop->assoc_len);
> +
> + atomic_inc(&ses_ptr->refcnt);
> +
> + if (cop->eop == COP_ENCRYPT)
> + ret = crypto_aead_encrypt(req);
> + else
> + ret = crypto_aead_decrypt(req);
> +
> + switch (ret) {
> + case 0:
> + if (!iocb)
> + atomic_dec(&result->opcnt);
> + break;
> + case -EINPROGRESS:
> + case -EBUSY:
> + if (iocb) {
> + CDPRINTK(2, KERN_INFO,
> + "Async Call AEAD:Returning Now\n");
> + return -EIOCBQUEUED;
> + }
> + ret = wait_for_completion_interruptible(
> + &result->crypto_completion);
> + if (!ret)
> + ret = result->err;
> + if (!ret) {
> + INIT_COMPLETION(result->crypto_completion);
> + break;
> + }
I.e. let's suppose it was interrupted here, while crypto driver performs
a processing on given pages.
> + /* fall through */
> + default:
> + printk(KERN_ERR PFX "sid %p enc/dec failed error %d\n",
> + ses_ptr, -ret);
> + if (!iocb)
> + atomic_dec(&result->opcnt);
> + break;
> + }
> +
> + if (nopin && !ret) {
> + if (copy_to_user(dst, data, enc ? bufsize + authsize :
> + bufsize - authsize))
> + printk(KERN_ERR PFX
> + "failed to copy encrypted data "
> + "to user space\n");
> + CD_HEXDUMP(data, enc ? bufsize + authsize :
> + bufsize - authsize);
> + }
> +
> + /* Check if last reference */
> + if (atomic_dec_and_test(&ses_ptr->refcnt))
> + cryptodev_destroy_session(ses_ptr);
> + if (dst_flag)
> + cryptodev_release_pages(result->dpages, nr_dpages);
> +out_spages:
> + cryptodev_release_pages(result->spages, nr_spages);
Now you have freed pages, which are still accessible by the request
crypto processing code somewhere in the underlying driver.
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
