Re: [PATCH net 1/5] net: tls: handle -EBUSY on async encrypt/decrypt requests

2023-09-12, 12:43:32 +0800, Herbert Xu wrote:

On Fri, Sep 08, 2023 at 05:55:59PM +0200, Sabrina Dubroca wrote:

quoted

Uh, ok, I didn't know that, thanks for explaining. When I was fixing
this code I couldn't find a mention of what the expectations for
MAY_BACKLOG are. Could you add a comment describing this in the
headers (either for #define CRYPTO_TFM_REQ_MAY_BACKLOG or
aead_request_set_callback, wherever is more appropriate). MAY_BACKLOG
is used by both tls and tipc (talking only about networking) and
neither seem to respect this need to back off.

Patches are welcome :)

Ok. I thought it'd be better if you wrote that patch since if I write
it, it'll be a c/p (or rephrase) of what you wrote. But fine, I'll go
ahead and do that :)

A bit of history: at the beginning we always dropped requests
that we couldn't queue because the only user was IPsec so this
is the expected behaviour.

When storage crypto support was added there was a need for reliable
handling of resource constraints so that's why MAY_BACKLOG was added.
However, the expectation is obviously that you must stop sending new
requests once you run into the resource constraint.

quoted

Jakub, I guess we should drop the CRYPTO_TFM_REQ_MAY_BACKLOG for net,
and maybe consider adding it back (with the back off) in
net-next. Probably not urgent considering that nobody seems to have
run into this bug so far.

I think that would be the prudent action.

We'd have to do pretty much what Jakub suggested [1] (handle ENOSPC by
waiting for all current requests) and then resubmit the failed
request. So I think keeping the MAY_BACKLOG flag and handling EBUSY
this way is simpler. With this, we send one request to the backlog,
then we wait until the queue drains.

[1] https://lore.kernel.org/netdev/20230908142602.2ced0631@kernel.org/ (local)

-- 
Sabrina