Hello,
I'm digging in crypto/algapi.c , in the crypto_enqueue_request() function. I
don't quite understand how the backlog mechanism should work. According to [1],
I suspect it should limit the amount of requests in the queue to $max_qlen and
allow one more request to be enqueued into the $backlog ; and if there is more
requests than $max_qlen, it should start dropping the requests ?
Inspecting the code, I see these situations:
1) qlen < max_qlen
-> The request is enqueued, qlen is increased , returns -EINPROGRESS
2) qlen >= max_qlen
-> The crypto_enqueue_request() returns -EBUSY in this case
2a) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG unset
=> Request is not enqueued , qlen is not increased
2b) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG set
=> Request is enqueued , qlen is increased
Therefore, I suspect if 2b) case happens repeatedly, the queue will grow
indefinitelly. I'm not sure if this can happen, but I suspect that's really
possible. My understanding is that the driver will call crypto_enqueue_request()
and propagate it's return value to the upper layers. They upper layet can
generate such a request that has CRYPTO_TFM_REQ_MAY_BACKLOG set .
But how shall the upper layers handle the -EBUSY return value from the crypto
API calls? Shall they stop saturating the crypto API with requests ? How can the
upper layers determine when can they resume sending crypto requests ? Shall they
just try calling crypto_enqueue_request() again to see if it still returns -
EBUSY ?
Sorry if this is an obvious question and thanks for the help!
[1] http://permalink.gmane.org/gmane.linux.kernel.cryptoapi/735
Best regards,
Marek Vasut
Hello again,
> Hello,
>
> I'm digging in crypto/algapi.c , in the crypto_enqueue_request() function.
> I don't quite understand how the backlog mechanism should work. According
> to [1], I suspect it should limit the amount of requests in the queue to
> $max_qlen and allow one more request to be enqueued into the $backlog ;
> and if there is more requests than $max_qlen, it should start dropping the
> requests ?
>
> Inspecting the code, I see these situations:
> 1) qlen < max_qlen
> -> The request is enqueued, qlen is increased , returns -EINPROGRESS
> 2) qlen >= max_qlen
> -> The crypto_enqueue_request() returns -EBUSY in this case
> 2a) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG unset
> => Request is not enqueued , qlen is not increased
> 2b) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG set
> => Request is enqueued , qlen is increased
>
> Therefore, I suspect if 2b) case happens repeatedly, the queue will grow
> indefinitelly. I'm not sure if this can happen, but I suspect that's really
> possible. My understanding is that the driver will call
> crypto_enqueue_request() and propagate it's return value to the upper
> layers. They upper layet can generate such a request that has
> CRYPTO_TFM_REQ_MAY_BACKLOG set .
>
> But how shall the upper layers handle the -EBUSY return value from the
> crypto API calls? Shall they stop saturating the crypto API with requests
> ? How can the upper layers determine when can they resume sending crypto
> requests ? Shall they just try calling crypto_enqueue_request() again to
> see if it still returns - EBUSY ?
There is one more thing which I don't quite understand with regards to the
queues, it's this pattern one can see in every second crypto driver (I took this
from bfin_crc.c):
302 struct crypto_async_request *async_req, *backlog;
[...]
311 spin_lock_irqsave(&crc->lock, flags);
[...]
318 backlog = crypto_get_backlog(&crc->queue);
319 async_req = crypto_dequeue_request(&crc->queue);
320 if (async_req)
321 crc->busy = 1;
322 spin_unlock_irqrestore(&crc->lock, flags);
323
324 if (!async_req)
325 return ret;
326
327 if (backlog)
328 backlog->complete(backlog, -EINPROGRESS);
So, we call backlog->complete() , but who did actually ever process the request
stored in the queue->backlog ? To me, it seems like the request was never
processed, but we complete it.
I think this will work in case the ->backlog == ->list , since in that case we
will basically complete the currently dequeued request. Will this also work in
case ->backlog != ->list (that is, in case the qlen overflew max_qlen)? I'm not
sure.
Can you please help me understand this ?
Thank you!
Best regards,
Marek Vasut
On Mon, May 26, 2014 at 05:58:19PM +0200, Marek Vasut wrote:
> Hello,
>
> I'm digging in crypto/algapi.c , in the crypto_enqueue_request() function. I
> don't quite understand how the backlog mechanism should work. According to [1],
> I suspect it should limit the amount of requests in the queue to $max_qlen and
> allow one more request to be enqueued into the $backlog ; and if there is more
> requests than $max_qlen, it should start dropping the requests ?
>
> Inspecting the code, I see these situations:
> 1) qlen < max_qlen
> -> The request is enqueued, qlen is increased , returns -EINPROGRESS
> 2) qlen >= max_qlen
> -> The crypto_enqueue_request() returns -EBUSY in this case
> 2a) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG unset
> => Request is not enqueued , qlen is not increased
> 2b) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG set
> => Request is enqueued , qlen is increased
The queue is per-tfm. In this case you never want to share a
tfm between two users that do not agree on whether to have backlog
or not.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Mon, May 26, 2014 at 06:16:58PM +0200, Marek Vasut wrote:
>
> So, we call backlog->complete() , but who did actually ever process the request
> stored in the queue->backlog ? To me, it seems like the request was never
> processed, but we complete it.
Nobody. This is meant to indicate to the user that the queue now has
space for submission. The backlogged request must be resubmitted.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert:
It is nice to know that the request queue is per tfm. We are currently on android-3.10. I believe all the drivers under drivers/crypto/ don't follow this rule. At the minimum, our driver qcrypto does not. Other than backlogging function, does it break anything? How critical is it to fix the driver soon?
Thanks
Regards,
Chemin
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Herbert Xu
Sent: Friday, May 30, 2014 5:31 AM
To: Marek Vasut
Cc: [email protected]; Arnd Bergmann; Pantelis Antoniou
Subject: Re: [QUESTION] Crypto queue handling
On Mon, May 26, 2014 at 05:58:19PM +0200, Marek Vasut wrote:
> Hello,
>
> I'm digging in crypto/algapi.c , in the crypto_enqueue_request()
> function. I don't quite understand how the backlog mechanism should
> work. According to [1], I suspect it should limit the amount of
> requests in the queue to $max_qlen and allow one more request to be
> enqueued into the $backlog ; and if there is more requests than $max_qlen, it should start dropping the requests ?
>
> Inspecting the code, I see these situations:
> 1) qlen < max_qlen
> -> The request is enqueued, qlen is increased , returns
> -EINPROGRESS
> 2) qlen >= max_qlen
> -> The crypto_enqueue_request() returns -EBUSY in this case
> 2a) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG unset
> => Request is not enqueued , qlen is not increased
> 2b) request->flags has CRYPTO_TFM_REQ_MAY_BACKLOG set
> => Request is enqueued , qlen is increased
The queue is per-tfm. In this case you never want to share a tfm between two users that do not agree on whether to have backlog or not.
Cheers,
--
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, May 30, 2014 at 01:33:06PM +0000, Hsieh, Che-Min wrote:
>
> It is nice to know that the request queue is per tfm. We are currently on android-3.10. I believe all the drivers under drivers/crypto/ don't follow this rule. At the minimum, our driver qcrypto does not. Other than backlogging function, does it break anything? How critical is it to fix the driver soon?
Sorry I don't know what I was smoking when I said that but it's
bollocks :)
Marek is quite correct that MAY_BACKLOG users can indefinitely
add to the backlog queue. However, it is meant to be limited
to one entry per-tfm.
This is because the user is supposed to back off once they get
EBUSY, until they're notified once the backlog entry is popped
off (but not processed, it must be resubmitted).
We don't really do any checking right now as there aren't many
MAY_BACKLOG users. But if it becomes a problem we can always
add a check to ensure each tfm does not add more than one backlog
request.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Tue, Nov 11, 2014 at 08:04:03PM +0200, Nicolae Rosia wrote:
> On Fri, May 30, 2014 at 4:41 PM, Herbert Xu <[email protected]>
> wrote:
>
> > [...]
> > This is because the user is supposed to back off once they get
> > EBUSY, until they're notified once the backlog entry is popped
> > off (but not processed, it must be resubmitted).
> > [...]
> >
>
> I'm trying to understand the backlog mechanism and after reading the source
> of crypto_enqueue_request and Stephan Mueller's example [1], my
> understanding is that the user does not have to queue again the backlogged
> request because when it returned -EBUSY it was added to the
> crypto_queue->list. Am I missing something?
You're right, the backlogged request does not need to be resubmitted.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt