netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] tls: rx: fix return value for async crypto
@ 2023-02-27 18:12 Jakub Kicinski
  2023-02-28  6:53 ` [EXT] " Gaurav Jain
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jakub Kicinski @ 2023-02-27 18:12 UTC (permalink / raw)
  To: davem
  Cc: netdev, edumazet, pabeni, gaurav.jain, Jakub Kicinski, borisp,
	john.fastabend

Gaurav reports that TLS Rx is broken with async crypto
accelerators. The commit under fixes missed updating
the retval byte counting logic when updating how records
are stored. Even tho both before and after the change
'decrypted' was updated inside the main loop, it was
completely overwritten when processing the async
completions. Now that the rx_list only holds
non-zero-copy records we need to add, not overwrite.

Reported-and-bisected-by: Gaurav Jain <gaurav.jain@nxp.com>
Fixes: cbbdee9918a2 ("tls: rx: async: don't put async zc on the list")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217064
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: borisp@nvidia.com
CC: john.fastabend@gmail.com
---
 net/tls/tls_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 782d3701b86f..021d760f9133 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2127,7 +2127,7 @@ int tls_sw_recvmsg(struct sock *sk,
 		else
 			err = process_rx_list(ctx, msg, &control, 0,
 					      async_copy_bytes, is_peek);
-		decrypted = max(err, 0);
+		decrypted += max(err, 0);
 	}
 
 	copied += decrypted;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: [EXT] [PATCH net] tls: rx: fix return value for async crypto
  2023-02-27 18:12 [PATCH net] tls: rx: fix return value for async crypto Jakub Kicinski
@ 2023-02-28  6:53 ` Gaurav Jain
  2023-02-28 11:24 ` Simon Horman
  2023-02-28 22:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Gaurav Jain @ 2023-02-28  6:53 UTC (permalink / raw)
  To: Jakub Kicinski, davem; +Cc: netdev, edumazet, pabeni, borisp, john.fastabend

Tested-by: Gaurav Jain <gaurav.jain@nxp.com>

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, February 27, 2023 11:42 PM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; edumazet@google.com; pabeni@redhat.com;
> Gaurav Jain <gaurav.jain@nxp.com>; Jakub Kicinski <kuba@kernel.org>;
> borisp@nvidia.com; john.fastabend@gmail.com
> Subject: [EXT] [PATCH net] tls: rx: fix return value for async crypto
> 
> Caution: EXT Email
> 
> Gaurav reports that TLS Rx is broken with async crypto accelerators. The commit
> under fixes missed updating the retval byte counting logic when updating how
> records are stored. Even tho both before and after the change 'decrypted' was
> updated inside the main loop, it was completely overwritten when processing
> the async completions. Now that the rx_list only holds non-zero-copy records
> we need to add, not overwrite.
> 
> Reported-and-bisected-by: Gaurav Jain <gaurav.jain@nxp.com>
> Fixes: cbbdee9918a2 ("tls: rx: async: don't put async zc on the list")
> Link:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.k
> ernel.org%2Fshow_bug.cgi%3Fid%3D217064&data=05%7C01%7Cgaurav.jain%4
> 0nxp.com%7C77f145ef30784d369bca08db18ee26ab%7C686ea1d3bc2b4c6fa92
> cd99c5c301635%7C0%7C0%7C638131183344515410%7CUnknown%7CTWFpbG
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D%7C3000%7C%7C%7C&sdata=PQE9yFQ85lN8RvbgLwLIZmRbCzZOVcF8VB
> MEWTgi%2B4Q%3D&reserved=0
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: borisp@nvidia.com
> CC: john.fastabend@gmail.com
> ---
>  net/tls/tls_sw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
> 782d3701b86f..021d760f9133 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2127,7 +2127,7 @@ int tls_sw_recvmsg(struct sock *sk,
>                 else
>                         err = process_rx_list(ctx, msg, &control, 0,
>                                               async_copy_bytes, is_peek);
> -               decrypted = max(err, 0);
> +               decrypted += max(err, 0);
>         }
> 
>         copied += decrypted;
> --
> 2.39.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tls: rx: fix return value for async crypto
  2023-02-27 18:12 [PATCH net] tls: rx: fix return value for async crypto Jakub Kicinski
  2023-02-28  6:53 ` [EXT] " Gaurav Jain
@ 2023-02-28 11:24 ` Simon Horman
  2023-02-28 22:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2023-02-28 11:24 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, gaurav.jain, borisp, john.fastabend

On Mon, Feb 27, 2023 at 10:12:01AM -0800, Jakub Kicinski wrote:
> Gaurav reports that TLS Rx is broken with async crypto
> accelerators. The commit under fixes missed updating
> the retval byte counting logic when updating how records
> are stored. Even tho both before and after the change
> 'decrypted' was updated inside the main loop, it was
> completely overwritten when processing the async
> completions. Now that the rx_list only holds
> non-zero-copy records we need to add, not overwrite.
> 
> Reported-and-bisected-by: Gaurav Jain <gaurav.jain@nxp.com>
> Fixes: cbbdee9918a2 ("tls: rx: async: don't put async zc on the list")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217064
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Simon Horman <simon.horman@corigine.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tls: rx: fix return value for async crypto
  2023-02-27 18:12 [PATCH net] tls: rx: fix return value for async crypto Jakub Kicinski
  2023-02-28  6:53 ` [EXT] " Gaurav Jain
  2023-02-28 11:24 ` Simon Horman
@ 2023-02-28 22:30 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-02-28 22:30 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem, netdev, edumazet, pabeni, gaurav.jain, borisp, john.fastabend

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 27 Feb 2023 10:12:01 -0800 you wrote:
> Gaurav reports that TLS Rx is broken with async crypto
> accelerators. The commit under fixes missed updating
> the retval byte counting logic when updating how records
> are stored. Even tho both before and after the change
> 'decrypted' was updated inside the main loop, it was
> completely overwritten when processing the async
> completions. Now that the rx_list only holds
> non-zero-copy records we need to add, not overwrite.
> 
> [...]

Here is the summary with links:
  - [net] tls: rx: fix return value for async crypto
    https://git.kernel.org/netdev/net/c/4d42cd6bc2ac

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-02-28 22:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-27 18:12 [PATCH net] tls: rx: fix return value for async crypto Jakub Kicinski
2023-02-28  6:53 ` [EXT] " Gaurav Jain
2023-02-28 11:24 ` Simon Horman
2023-02-28 22:30 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).