From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (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 99F87E4C5 for ; Tue, 28 Mar 2023 09:43:19 +0000 (UTC) Received: by mail-wm1-f54.google.com with SMTP id r19-20020a05600c459300b003eb3e2a5e7bso6970290wmo.0 for ; Tue, 28 Mar 2023 02:43:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679996598; x=1682588598; 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=UpCifbAQFmnrwNJzCRQqXbs9XH+eML8HHz8+PQTc/B4=; b=YyghB2uIHOF/JzzKHJ/Y7iJ+7IBWS7IMMkwgpFjCztws30mOKg0Jcefzq55lEB51D+ Zeedulehtc5A3M+qp0sCeppkx0HvMGoNcfeXsN9fQBNS2liLPDLtlVPnHv4G4a+9jEeN u7dQBfqxQ7NnjD982JqO2aMrPTnJLHTo6YIaLFtSDqrz6RlQJHayDZoC/0QVucgd1CXH xCnQpktuA3J9AKtLfDRDPOWxgXIfOZxnx7o4+TRFHu9wNdwjCOl0L+hfmIgCZ6/duEct pPoKB53LiUE46DDXgiyuBEHTYDtJMoJKkxE/vFbhJEhOoS7Wa1+10BcJjIBvQzZ/O8eO 3duQ== X-Gm-Message-State: AO0yUKWo1xY/Fdo5sBhVzswRHLI/P9FlayKN6uoZqpUVKwWRb8e3xhdD v8KH4z+aHaaoOq8YXR5P3dQ= X-Google-Smtp-Source: AK7set/rCJ0qGQGvAy44YkfwAdUspx7k76llslWAzbJ24bdJ3F8akoOKnTMhuktllVt1cevz2rmsJw== X-Received: by 2002:a05:600c:1d1f:b0:3e9:b564:fae4 with SMTP id l31-20020a05600c1d1f00b003e9b564fae4mr12895102wms.0.1679996597804; Tue, 28 Mar 2023 02:43:17 -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 iv11-20020a05600c548b00b003ef6ceedf14sm7796484wmb.38.2023.03.28.02.43.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 28 Mar 2023 02:43:17 -0700 (PDT) Message-ID: <3962f9d9-2ee0-2065-827f-ca0cda881271@grimberg.me> Date: Tue, 28 Mar 2023 12:43:16 +0300 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.9.0 Subject: Re: [PATCH 15/18] nvmet-tcp: enable TLS handshake upcall Content-Language: en-US To: Hannes Reinecke , Chuck Lever III 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> <3ad50302-6b3c-6d6a-669e-0b11c2df6a6b@suse.de> <7c3da604-251f-209e-db6d-d1bee5a838c5@suse.de> <8d0ddb2d-0b4e-e782-6507-a1d61b5d53f4@grimberg.me> <8fa65f04-7e82-77a6-00de-f51d20138ece@suse.de> From: Sagi Grimberg In-Reply-To: <8fa65f04-7e82-77a6-00de-f51d20138ece@suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit >>> But there might be the odd case where the data_ready >>> callback is invoked after kTLS has been started but before >>> we're setting it to nvmet_tcp_data_ready(). >> >> What does nvmet_tcp_data_ready has to do with it? >> You are changing nvmet_tcp_listen_data_ready. >> > This is not a 'race; >>> Then we cannot guarantee that sk_user_data really is set >>> to the 'queue' pointer, so we should skip the function. >> >> nvme_tcp_listen_data_ready was invoked, then sk->sk_data_ready >> is nvme_tcp_listen_data_ready. Are you referring to the case >> where the callback has changed before the read lock ? >> > No. The callback is changed when the userspace daemon starts ktls, as > that will set sk->sk_data_ready to tls_data_ready(). > > So before the userspace upcall we have the chain: > > sk_data_ready -> nvmet_tcp_listen_data_ready() > > and after a (successful) upcall we have the chain > > sk_data_ready -> tls_data_ready >   -> nvmet_tcp_listen_data_ready() > > once we set our callback we have > > sk_data_ready -> nvmet_tcp_data_ready >   -> tls_data_ready >     -> nvmet_tcp_listen_data_ready > > Calling into nvmet_tcp_listen_data_ready() is pointless here, > but we cannot remove the callback either. So we should to deactivate > it to avoid any side effects. Thanks for the explanation. First, I would like to understand how is this different from svc. Second, we don't want nvmet_tcp_listen_data_ready keep being called from the datapath, especially as it is taking a read lock, for absolutly no good reason. Why don't we restore the original socket callback before we trigger the handshake? then we wait for the handshake to complete, and then store nvmet_tcp_data_ready once it is done?