Re: [PATCH net-next v3 7/7] tls: rx: do not use the standard strparser

From: Jakub Kicinski <kuba@kernel.org>
To: Tariq Toukan <ttoukan.linux@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
	pabeni@redhat.com, borisp@nvidia.com, john.fastabend@gmail.com,
	maximmi@nvidia.com, tariqt@nvidia.com, vfedorenko@novek.ru,
	Gal Pressman <gal@nvidia.com>, Saeed Mahameed <saeedm@nvidia.com>
Subject: Re: [PATCH net-next v3 7/7] tls: rx: do not use the standard strparser
Date: Mon, 13 Mar 2023 11:22:10 -0700	[thread overview]
Message-ID: <20230313112210.71905e2d@kernel.org> (raw)
In-Reply-To: <89086da6-b559-f6c0-d73a-6c73ff74dff5@gmail.com>

On Sun, 12 Mar 2023 19:59:57 +0200 Tariq Toukan wrote:
> On 09/03/2023 19:54, Jakub Kicinski wrote:
> > On Thu, 9 Mar 2023 17:15:26 +0200 Tariq Toukan wrote:  
> >> A few fixes were introduced for this patch, but it seems to still cause
> >> issues.
> >>
> >> I'm running simple client/server test with wrk and nginx and TLS RX
> >> device offload on.
> >> It fails with TlsDecryptError on the client side for the large file
> >> (256000b), while succeeding for the small one (10000b). See repro
> >> details below.
> >>
> >> I narrowed the issue down to this offending patch, by applying a few
> >> reverts (had to solve trivial conflicts):  
> > 
> > What's the sequence of records in terms of being offloaded vs fall back?
> > Could you whip up a simple ring buffer to see if previous records were
> > offloaded and what the skb geometries where?  
> 
> Interesting. All records go through the sw fallback.
> 
> Command:
> $ wrk_openssl_3_0_0 -b2.2.2.2 -t1 -c1 -d2 --timeout 5s 
> https://2.2.2.3:20443/256000b.img

Is wrk_openssl_3_0_0 a nginx command? Any CX6 DX card can do this?

> Debug code:
> @@ -1712,8 +1723,13 @@ static int tls_rx_one_record(struct sock *sk, 
> struct msghdr *msg,
>          int err;
> 
>          err = tls_decrypt_device(sk, msg, tls_ctx, darg);
> -       if (!err)
> +       if (!err) {
>                  err = tls_decrypt_sw(sk, tls_ctx, msg, darg);
> +               printk("sk: %p, tls_decrypt_sw, err = %d\n", sk, err);
> +       } else {
> +               printk("sk: %p, tls_decrypt_device, err = %d\n", sk, err);
> +       }
> +       skb_dump(KERN_ERR, darg->skb, false);
>          if (err < 0)
>                  return err;
> 
> dmesg output including skb geometries is attached.

Hm, could you add to the debug the addresses of the fragments 
(and decrypted status) of the data queued to TCP by the driver?
And then the frag addresses in skb_dump() ?

tls_decrypt_sw() will also get used in partially decrypted records, 
right?