public inbox for [email protected]
 help / color / mirror / Atom feed
From: Sascha Hauer <[email protected]>
To: Jens Axboe <[email protected]>
Cc: Boris Pismenny <[email protected]>,
	[email protected], John Fastabend <[email protected]>,
	[email protected], [email protected],
	[email protected], Jakub Kicinski <[email protected]>,
	Pavel Begunkov <[email protected]>
Subject: Re: Problem with io_uring splice and KTLS
Date: Mon, 16 Oct 2023 09:26:46 +0200	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On Fri, Oct 13, 2023 at 07:45:55AM -0600, Jens Axboe wrote:
> On 10/12/23 11:47 PM, Sascha Hauer wrote:
> > On Thu, Oct 12, 2023 at 07:45:07PM -0600, Jens Axboe wrote:
> >> On 10/12/23 7:34 AM, Sascha Hauer wrote:
> >>> In case you don't have encryption hardware you can create an
> >>> asynchronous encryption module using cryptd. Compile a kernel with
> >>> CONFIG_CRYPTO_USER_API_AEAD and CONFIG_CRYPTO_CRYPTD and start the
> >>> webserver with the '-c' option. /proc/crypto should then contain an
> >>> entry with:
> >>>
> >>>  name         : gcm(aes)
> >>>  driver       : cryptd(gcm_base(ctr(aes-generic),ghash-generic))
> >>>  module       : kernel
> >>>  priority     : 150
> >>
> >> I did a bit of prep work to ensure I had everything working for when
> >> there's time to dive into it, but starting it with -c doesn't register
> >> this entry. Turns out the bind() in there returns -1/ENOENT.
> > 
> > Yes, that happens here as well, that's why I don't check for the error
> > in the bind call. Nevertheless it has the desired effect that the new
> > algorithm is registered and used from there on. BTW you only need to
> > start the webserver once with -c. If you start it repeatedly with -c a
> > new gcm(aes) instance is registered each time.
> 
> Gotcha - I wasn't able to trigger the condition, which is why I thought
> perhaps I was missing something.
> 
> Can you try the below patch and see if that makes a difference? I'm not
> quite sure why it would since you said it triggers with DEFER_TASKRUN as
> well, and for that kind of notification, you should never hit the paths
> you have detailed in the debug patch.

I can confirm that this patch makes it work for me. I tested with both
software cryptd and also with my original CAAM encryption workload.
IORING_SETUP_SINGLE_ISSUER | IORING_SETUP_DEFER_TASKRUN is not needed.
Both my simple webserver and the original C++ Webserver from our
customer are now working without problems.

Do you think there is a chance getting this change upstream? I'm a bit
afraid the code originally uses signal_pending() instead of
task_sigpending() for a good reason.

Sascha

> 
> diff --git a/net/core/stream.c b/net/core/stream.c
> index f5c4e47df165..a9a196587254 100644
> --- a/net/core/stream.c
> +++ b/net/core/stream.c
> @@ -67,7 +67,7 @@ int sk_stream_wait_connect(struct sock *sk, long *timeo_p)
>  			return -EPIPE;
>  		if (!*timeo_p)
>  			return -EAGAIN;
> -		if (signal_pending(tsk))
> +		if (task_sigpending(tsk))
>  			return sock_intr_errno(*timeo_p);
>  
>  		add_wait_queue(sk_sleep(sk), &wait);
> @@ -103,7 +103,7 @@ void sk_stream_wait_close(struct sock *sk, long timeout)
>  		do {
>  			if (sk_wait_event(sk, &timeout, !sk_stream_closing(sk), &wait))
>  				break;
> -		} while (!signal_pending(current) && timeout);
> +		} while (!task_sigpending(current) && timeout);
>  
>  		remove_wait_queue(sk_sleep(sk), &wait);
>  	}
> @@ -134,7 +134,7 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
>  			goto do_error;
>  		if (!*timeo_p)
>  			goto do_eagain;
> -		if (signal_pending(current))
> +		if (task_sigpending(current))
>  			goto do_interrupted;
>  		sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
>  		if (sk_stream_memory_free(sk) && !vm_wait)
> 
> -- 
> Jens Axboe
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2023-10-16  7:26 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-10 14:19 Problem with io_uring splice and KTLS Sascha Hauer
2023-10-10 14:28 ` Jens Axboe
2023-10-11 11:21   ` Sascha Hauer
2023-10-12 13:34   ` Sascha Hauer
2023-10-13  1:13     ` Jens Axboe
2023-10-13  1:45     ` Jens Axboe
2023-10-13  5:47       ` Sascha Hauer
2023-10-13 13:45         ` Jens Axboe
2023-10-16  7:26           ` Sascha Hauer [this message]
2023-10-16 13:17             ` Jens Axboe
2023-10-17 11:51               ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox