From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3A65B23C3 for ; Thu, 23 Mar 2023 07:21:06 +0000 (UTC) Received: by mail-wr1-f51.google.com with SMTP id v1so13273627wrv.1 for ; Thu, 23 Mar 2023 00:21:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679556064; x=1682148064; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=oOhWx5hqgjPuc55ynmMAxZ4Nttm12OYOh7+N2OfmB+4=; b=UE2HCdyIGOBiYvroZSn+/KXXHd6axUMYTLFLqPMKnVnJN7jKY5VksXiWb2vf17yRAP HFNHCpP/16GBve3TcR6SR4US0K37mNcWXU2aLAu7YOYdAwuJexYkEfYjUHz2xfuEwT+N NRtcz0kmH1Zfm6X0IFxEYsbCWbu5Qb3pl7zy6AHSDiJp8RKSU0TC1kcIP0qDd0WRdjYi 6MXYalchiErwcE7oTR9M7xApBWgSL820HiR2fEZ0iG+81GXlPYuhfL7jfKxqFleKNeSi fY2uxhEQkSmciffsi/Aqpq/J+15s3RdfDpiMc1Uvc6GK/SADSdeqB6u2vtQlQpSMObUH MbGw== X-Gm-Message-State: AAQBX9f3vbChO2AVLGVpyFQXukSmstFeUD+STecWC6jDV65hQSKbubtm Qw92PKQX0FVVVqZN8zoQWo0= X-Google-Smtp-Source: AKy350YqnStmgqqn0t1BZqc6IF7s4ryZPEhE56GlQconW8MZ82zQ0ro2hjnIIHgAy4QZYOFd1DuWzw== X-Received: by 2002:a5d:404d:0:b0:2db:43ed:1bb6 with SMTP id w13-20020a5d404d000000b002db43ed1bb6mr1472663wrp.3.1679556064296; Thu, 23 Mar 2023 00:21:04 -0700 (PDT) Received: from [192.168.64.192] (bzq-219-42-90.isdn.bezeqint.net. [62.219.42.90]) by smtp.gmail.com with ESMTPSA id p14-20020a5d48ce000000b002d45575643esm13442756wrs.43.2023.03.23.00.21.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Mar 2023 00:21:04 -0700 (PDT) Message-ID: Date: Thu, 23 Mar 2023 09:21:02 +0200 Precedence: bulk X-Mailing-List: kernel-tls-handshake@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH 15/18] nvmet-tcp: enable TLS handshake upcall Content-Language: en-US To: Chuck Lever III , Hannes Reinecke Cc: Christoph Hellwig , Keith Busch , "linux-nvme@lists.infradead.org" , Chuck Lever , "kernel-tls-handshake@lists.linux.dev" References: <20230321124325.77385-1-hare@suse.de> <20230321124325.77385-16-hare@suse.de> <6f69983c-17bd-dd72-b941-95f55db10180@grimberg.me> <68AA3629-6B88-4098-AA03-BEDF614F8D0B@oracle.com> From: Sagi Grimberg In-Reply-To: <68AA3629-6B88-4098-AA03-BEDF614F8D0B@oracle.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit >>>>>> The 'data_ready' call might happen at any time after the 'accept' call and us calling into userspace. >>>>>> In particular we have this flow of control: >>>>>> >>>>>> 1. Kernel: accept() >>>>>> 2. Kernel: handshake request >>>>>> 3. Userspace: read data from socket >>>>>> 4. Userspace: tls handshake >>>>>> 5. Kernel: handshake complete >>>>>> >>>>>> If the 'data_ready' event occurs between 1. and 3. userspace wouldn't know that something has happened, and will be sitting there waiting for data which is already present. >>>>> >>>>> Umm, doesn't userspace read from the socket once we trigger the upcall? >>>>> it should. But I still don't understand what is the difference between >>>>> us waiking up userspace, from the default sock doing the same? >>>>> >>>> No, it doesn't (or, rather, can't). >>>> After processing 'accept()' (from the kernel code) data might already be present (after all, why would we get an 'accept' call otherwise?). >>>> But the daemon has not been started up (yet); that's only done in >>>> step 3). But 'data_ready' has already been called, so by the time userland is able to do a 'read()' on the socket it won't be seeing anything. >>> Not sure I understand. if data exists, userspace will read from the >>> socket and get data, whenever that is. > >> That's what I thought, too. >> But then the userspace daemon just sat there doing nothing. > > I haven't been following this discussion in detail, but > if the kernel disables the normal TCP data_ready callback, > then user space won't get any data. That's why SunRPC's > data_ready calls the previous sk_data_ready and then shunts > its own data_ready callback during handshakes. Without that > call to the old sk_data_ready, the user space endpoint won't > see any received data. Yes that is understood. But the solution that Hannes proposed was to introduce nvmet_tcp_tls_data_ready which is overriding the default sock data_ready and does pretty much the same thing. The reason is that today nvmet_tcp_listen_data_ready schedules accept and then pretty much immediately replaces the socket data_ready to nvmet_tcp_data_ready. I think that a simpler solution was to make nvmet_tcp_listen_data_ready call port->data_ready (default socket stored data_ready), schedule the accept_work and only after the handshake bounce to userspace is completed, override the socket callbacks. Something like: -- static void nvmet_tcp_listen_data_ready(struct sock *sk) { struct nvmet_tcp_port *port; trace_sk_data_ready(sk); read_lock_bh(&sk->sk_callback_lock); port = sk->sk_user_data; if (!port) goto out; port->data_ready(sk); // trigger socket old data_ready if (sk->sk_state == TCP_LISTEN) queue_work(nvmet_wq, &port->accept_work); out: read_unlock_bh(&sk->sk_callback_lock); } ... static void nvmet_tcp_tls_handshake_done(void *data, int status, key_serial_t peerid) { ... queue->state = NVMET_TCP_Q_CONNECTING; nvmet_tcp_set_queue_sock(queue); // now own socket data_ready } --