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

[Sabrina Dubroca <sd@queasysnail.net>]

2023-09-12, 12:43:32 +0800, Herbert Xu wrote:
> On Fri, Sep 08, 2023 at 05:55:59PM +0200, Sabrina Dubroca wrote:
> >
> > 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.
> 
> > 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/

-- 
Sabrina