kernel-tls-handshake.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagi@grimberg.me>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Hannes Reinecke <hare@suse.de>, Christoph Hellwig <hch@lst.de>,
	Boris Pismenny <borisp@nvidia.com>,
	john.fastabend@gmail.com, Paolo Abeni <pabeni@redhat.com>,
	Keith Busch <kbusch@kernel.org>,
	linux-nvme@lists.infradead.org,
	Chuck Lever <chuck.lever@oracle.com>,
	kernel-tls-handshake@lists.linux.dev,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH 10/18] nvme-tcp: fixup send workflow for kTLS
Date: Mon, 3 Apr 2023 18:51:09 +0300	[thread overview]
Message-ID: <c7a07e1d-b300-dd1d-1be6-311666387820@grimberg.me> (raw)
In-Reply-To: <20230403075946.26ad71ee@kernel.org>


>>>> Some of the flags are call specific, others may be internal to the
>>>> networking stack (e.g. the DECRYPTED flag). Old protocols didn't do
>>>> any validation because people coded more haphazardly in the 90s.
>>>> This lack of validation is a major source of technical debt :(
>>>
>>> A-ha. So what is the plan?
>>> Should the stack validate flags?
>>> And should the rules for validating be the same for all protocols?
>>
>> MSG_SENDPAGE_NOTLAST is not an internal flag, I thought it was
>> essentially similar semantics to MSG_MORE but for sendpage. It'd
>> be great if this can be allowed in tls (again, at the very least
>> don't fail but continue as if it wasn't passed).
> 
> .. but.. MSG_SENDPAGE_NOTLAST is supported in TLS, isn't it?
> Why are we talking about it?

Ah, right.

What I'm assuming that Hannes is tripping on is that tls does
not accept when this flag is sent to sock_no_sendpage, which
is simply calling sendmsg. TLS will not accept this flag when
passed to sendmsg IIUC.

Today the rough logic in nvme send path is:

	if (more_coming(queue)) {
		flags = MSG_MORE | MSG_SENDPAGE_NOTLAST;
	} else {
		flags = MSG_EOR;
	}

	if (!sendpage_ok(page)) {
		kernel_sendpage();
	} else {
		sock_no_sendpage();
	}

This pattern (note that sock_no_sednpage was added later following bug
reports where nvme attempted to sendpage a slab allocated page), is
perfectly acceptable with normal sockets, but not with TLS.

So there are two options:
1. have tls accept MSG_SENDPAGE_NOTLAST in sendmsg (called from
    sock_no_sendpage)
2. Make nvme set MSG_SENDPAGE_NOTLAST only when calling
    kernel_sendpage and clear it when calling sock_no_sendpage

If you say that MSG_SENDPAGE_NOTLAST must be cleared when calling
sock_no_sendpage and it is a bug that it isn't enforced for normal tcp
sockets, then we need to change nvme, but I did not find
any documentation that indicates it, and right now, normal sockets
behave differently than tls sockets (wrt this flag in particular).

Hope this clarifies.

  reply	other threads:[~2023-04-03 15:51 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-29 13:59 [PATCHv2 00/18] nvme: In-kernel TLS support for TCP Hannes Reinecke
2023-03-29 13:59 ` [PATCH 01/18] nvme-keyring: register '.nvme' keyring and add CONFIG_NVME_TLS Hannes Reinecke
2023-03-29 14:49   ` Sagi Grimberg
2023-03-29 15:24     ` Hannes Reinecke
2023-03-29 15:04   ` Sagi Grimberg
2023-03-29 15:26     ` Hannes Reinecke
2023-03-30  8:53   ` Daniel Wagner
2023-03-30 14:38     ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 02/18] nvme-keyring: define a 'psk' keytype Hannes Reinecke
2023-03-29 13:59 ` [PATCH 03/18] nvme: add TCP TSAS definitions Hannes Reinecke
2023-03-29 13:59 ` [PATCH 04/18] nvme-tcp: add definitions for TLS cipher suites Hannes Reinecke
2023-03-29 13:59 ` [PATCH 05/18] net/tls: implement ->read_sock() Hannes Reinecke
2023-03-29 15:37   ` Sagi Grimberg
2023-03-29 15:41     ` Hannes Reinecke
2023-03-29 15:43       ` Sagi Grimberg
2023-03-29 15:44   ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 06/18] nvme/tcp: allocate socket file Hannes Reinecke
2023-03-29 15:57   ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 07/18] nvme-keyring: implement nvme_tls_psk_default() Hannes Reinecke
2023-03-29 15:35   ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 08/18] security/keys: export key_lookup() Hannes Reinecke
2023-03-29 13:59 ` [PATCH 09/18] nvme-tcp: enable TLS handshake upcall Hannes Reinecke
2023-03-30 12:54   ` Daniel Wagner
2023-03-30 12:59     ` Hannes Reinecke
2023-03-30 15:03   ` Sagi Grimberg
2023-03-30 17:16     ` Hannes Reinecke
2023-03-29 13:59 ` [PATCH 10/18] nvme-tcp: fixup send workflow for kTLS Hannes Reinecke
2023-03-30 15:24   ` Sagi Grimberg
2023-03-30 17:26     ` Hannes Reinecke
2023-03-31  5:49     ` Jakub Kicinski
2023-03-31  6:03       ` Hannes Reinecke
2023-04-03 12:20         ` Sagi Grimberg
2023-04-03 14:59           ` Jakub Kicinski
2023-04-03 15:51             ` Sagi Grimberg [this message]
2023-04-03 18:48               ` Jakub Kicinski
2023-04-03 22:36                 ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 11/18] nvme-tcp: control message handling for recvmsg() Hannes Reinecke
2023-03-30 15:25   ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 12/18] nvme-fabrics: parse options 'keyring' and 'tls_key' Hannes Reinecke
2023-03-30 15:33   ` Sagi Grimberg
2023-03-30 17:34     ` Hannes Reinecke
2023-04-03 12:24       ` Sagi Grimberg
2023-04-03 12:36         ` Hannes Reinecke
2023-04-03 13:07           ` Sagi Grimberg
2023-04-03 14:11             ` Hannes Reinecke
2023-04-03 16:13               ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 13/18] nvmet: make TCP sectype settable via configfs Hannes Reinecke
2023-03-30 16:07   ` Sagi Grimberg
2023-03-30 17:37     ` Hannes Reinecke
2023-04-03 12:31       ` Sagi Grimberg
2023-04-03 12:43         ` Hannes Reinecke
2023-03-29 13:59 ` [PATCH 14/18] nvmet-tcp: allocate socket file Hannes Reinecke
2023-03-30 16:08   ` Sagi Grimberg
2023-03-30 17:37     ` Hannes Reinecke
2023-03-29 13:59 ` [PATCH 15/18] nvmet-tcp: enable TLS handshake upcall Hannes Reinecke
2023-04-03 12:51   ` Sagi Grimberg
2023-04-03 14:05     ` Hannes Reinecke
2023-03-29 13:59 ` [PATCH 16/18] nvmet-tcp: rework sendpage for kTLS Hannes Reinecke
2023-04-03 12:52   ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 17/18] nvmet-tcp: control messages for recvmsg() Hannes Reinecke
2023-04-03 12:59   ` Sagi Grimberg
2023-03-29 13:59 ` [PATCH 18/18] nvmet-tcp: add configfs attribute 'param_keyring' Hannes Reinecke
2023-04-03 13:03   ` Sagi Grimberg
2023-04-03 14:13     ` Hannes Reinecke
2023-04-03 15:53       ` Sagi Grimberg
2023-04-14 10:30         ` Hannes Reinecke
2023-04-17 13:50           ` Sagi Grimberg
2023-04-17 14:01             ` Hannes Reinecke
2023-04-17 15:12               ` Sagi Grimberg
  -- strict thread matches above, loose matches on Subject: below --
2023-03-21 12:43 [RFC PATCH 00/18] nvme: In-kernel TLS support for TCP Hannes Reinecke
2023-03-21 12:43 ` [PATCH 10/18] nvme-tcp: fixup send workflow for kTLS Hannes Reinecke
2023-03-22  9:31   ` Sagi Grimberg
2023-03-22 10:08     ` Hannes Reinecke
2023-03-22 11:18       ` Sagi Grimberg

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 \
    --in-reply-to=c7a07e1d-b300-dd1d-1be6-311666387820@grimberg.me \
    --to=sagi@grimberg.me \
    --cc=borisp@nvidia.com \
    --cc=chuck.lever@oracle.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=john.fastabend@gmail.com \
    --cc=kbusch@kernel.org \
    --cc=kernel-tls-handshake@lists.linux.dev \
    --cc=kuba@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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;
as well as URLs for NNTP newsgroup(s).