| Message ID | 20240802114947.3984577-1-yiyang13@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [-next] crypto: testmgr - don't generate WARN for -EAGAIN | expand |
On Fri, Aug 02, 2024 at 11:49:47AM +0000, Yi Yang wrote: > Since commit 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET"), > The encryption and decryption using padata be failed when the CPU goes > online and offline. > We should try to re-encrypt or re-decrypt when -EAGAIN happens rather than > generate WARN. The unnecessary panic will occur when panic_on_warn set 1. > > Fixes: 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET") > Signed-off-by: Yi Yang <yiyang13@huawei.com> > --- > crypto/testmgr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) We should not expect Crypto API users to retry requests in this manner. If this is a reliability issue, perhaps padata should be performing the retry? Steffen? Thanks,
On Tue, Aug 06, 2024 at 01:59:41PM +0800, Herbert Xu wrote: > On Fri, Aug 02, 2024 at 11:49:47AM +0000, Yi Yang wrote: > > Since commit 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET"), > > The encryption and decryption using padata be failed when the CPU goes > > online and offline. > > We should try to re-encrypt or re-decrypt when -EAGAIN happens rather than > > generate WARN. The unnecessary panic will occur when panic_on_warn set 1. > > > > Fixes: 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET") > > Signed-off-by: Yi Yang <yiyang13@huawei.com> > > --- > > crypto/testmgr.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > We should not expect Crypto API users to retry requests in this > manner. > > If this is a reliability issue, perhaps padata should be performing > the retry? Steffen? If padata_do_parallel returns an error, it means it can't take the parallelization request. That is either because the instance gets replaced or it goes down. There is currently no infrastructure to queue requests on error, in particular not if it goes down.
On Thu, Aug 08, 2024 at 12:09:57PM +0200, Steffen Klassert wrote: > On Tue, Aug 06, 2024 at 01:59:41PM +0800, Herbert Xu wrote: > > On Fri, Aug 02, 2024 at 11:49:47AM +0000, Yi Yang wrote: > > > Since commit 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET"), > > > The encryption and decryption using padata be failed when the CPU goes > > > online and offline. > > > We should try to re-encrypt or re-decrypt when -EAGAIN happens rather than > > > generate WARN. The unnecessary panic will occur when panic_on_warn set 1. > > > > > > Fixes: 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET") > > > Signed-off-by: Yi Yang <yiyang13@huawei.com> > > > --- > > > crypto/testmgr.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > We should not expect Crypto API users to retry requests in this > > manner. > > > > If this is a reliability issue, perhaps padata should be performing > > the retry? Steffen? > > If padata_do_parallel returns an error, it means it can't take the > parallelization request. That is either because the instance gets > replaced or it goes down. There is currently no infrastructure > to queue requests on error, in particular not if it goes down. Maybe pcrypt can call the crypto layer directly without parallelization in that case.
On Thu, Aug 08, 2024 at 12:16:58PM +0200, Steffen Klassert wrote: > > Maybe pcrypt can call the crypto layer directly without > parallelization in that case. Yes that should work. Thanks,
On 2024/8/8 18:36, Herbert Xu wrote: > On Thu, Aug 08, 2024 at 12:16:58PM +0200, Steffen Klassert wrote: >> >> Maybe pcrypt can call the crypto layer directly without >> parallelization in that case. > > Yes that should work. > > Thanks, > Does this mean that the user needs to call the interface of the crypto layer in this case? rather than requiring the kernel to handle this. Thanks, --
On Thu, Aug 15, 2024 at 09:25:39AM +0800, yiyang (D) wrote: > > Does this mean that the user needs to call the interface of the crypto layer > in this case? rather than requiring the kernel to handle this. No it means that pcrypt should intercept the error and retry the request without going through padata. Could you please redo the patch through pcrypt? Thanks,
On 2024/8/15 10:06, Herbert Xu wrote: > On Thu, Aug 15, 2024 at 09:25:39AM +0800, yiyang (D) wrote: >> >> Does this mean that the user needs to call the interface of the crypto layer >> in this case? rather than requiring the kernel to handle this. > > No it means that pcrypt should intercept the error and retry the > request without going through padata. Could you please redo the > patch through pcrypt? > > Thanks, > Hi,Herbert,I sent the patch several days ago. I don't know whether you have received it. I hope you have time to look into it, and I am still looking forward to your reply. patch: https://lore.kernel.org/all/20241015020935.296691-1-yiyang13@huawei.com/ Best regards, Yiyang
diff --git a/crypto/testmgr.c b/crypto/testmgr.c index f02cb075bd68..15e0f5e4ba6f 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -5905,7 +5905,7 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask) } pr_warn("alg: self-tests for %s using %s failed (rc=%d)", alg, driver, rc); - WARN(rc != -ENOENT, + WARN(rc != -ENOENT && rc != -EAGAIN, "alg: self-tests for %s using %s failed (rc=%d)", alg, driver, rc); } else {
Since commit 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET"), The encryption and decryption using padata be failed when the CPU goes online and offline. We should try to re-encrypt or re-decrypt when -EAGAIN happens rather than generate WARN. The unnecessary panic will occur when panic_on_warn set 1. Fixes: 8f4f68e788c3 ("crypto: pcrypt - Fix hungtask for PADATA_RESET") Signed-off-by: Yi Yang <yiyang13@huawei.com> --- crypto/testmgr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)