[PATCH 18/18] nvmet-tcp: peek icreq before starting TLS
Sagi Grimberg
sagi at grimberg.me
Tue Sep 12 05:55:34 PDT 2023
On 9/12/23 14:48, Hannes Reinecke wrote:
> On 9/12/23 13:32, Sagi Grimberg wrote:
>>
>>
>> On 8/24/23 17:39, Hannes Reinecke wrote:
>>> Incoming connection might be either 'normal' NVMe-TCP connections
>>> starting with icreq or TLS handshakes. To ensure that 'normal'
>>> connections can still be handled we need to peek the first packet
>>> and only start TLS handshake if it's not an icreq.
>>> With that we can lift the restriction to always set TREQ to
>>> 'required' when TLS1.3 is enabled.
>>>
>>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>>> ---
>>> drivers/nvme/target/configfs.c | 25 +++++++++++---
>>> drivers/nvme/target/nvmet.h | 5 +++
>>> drivers/nvme/target/tcp.c | 61 +++++++++++++++++++++++++++++++---
>>> 3 files changed, 82 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/nvme/target/configfs.c
>>> b/drivers/nvme/target/configfs.c
>>> index b780ce049163..9eed6e6765ea 100644
>>> --- a/drivers/nvme/target/configfs.c
>>> +++ b/drivers/nvme/target/configfs.c
>>> @@ -198,6 +198,20 @@ static ssize_t nvmet_addr_treq_store(struct
>>> config_item *item,
>>> return -EINVAL;
>>> found:
>>> + if (port->disc_addr.trtype == NVMF_TRTYPE_TCP &&
>>> + port->disc_addr.tsas.tcp.sectype == NVMF_TCP_SECTYPE_TLS13) {
>>> + switch (nvmet_addr_treq[i].type) {
>>> + case NVMF_TREQ_NOT_SPECIFIED:
>>> + pr_debug("treq '%s' not allowed for TLS1.3\n",
>>> + nvmet_addr_treq[i].name);
>>> + return -EINVAL;
>>> + case NVMF_TREQ_NOT_REQUIRED:
>>> + pr_warn("Allow non-TLS connections while TLS1.3 is
>>> enabled\n");
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> + }
>>> treq |= nvmet_addr_treq[i].type;
>>> port->disc_addr.treq = treq;
>>> return count;
>>> @@ -410,12 +424,15 @@ static ssize_t nvmet_addr_tsas_store(struct
>>> config_item *item,
>>> nvmet_port_init_tsas_tcp(port, sectype);
>>> /*
>>> - * The TLS implementation currently does not support
>>> - * secure concatenation, so TREQ is always set to 'required'
>>> - * if TLS is enabled.
>>> + * If TLS is enabled TREQ should be set to 'required' per default
>>> */
>>> if (sectype == NVMF_TCP_SECTYPE_TLS13) {
>>> - treq |= NVMF_TREQ_REQUIRED;
>>> + u8 sc = nvmet_port_disc_addr_treq_secure_channel(port);
>>> +
>>> + if (sc == NVMF_TREQ_NOT_SPECIFIED)
>>> + treq |= NVMF_TREQ_REQUIRED;
>>> + else
>>> + treq |= sc;
>>> } else {
>>> treq |= NVMF_TREQ_NOT_SPECIFIED;
>>> }
>>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>>> index e35a03260f45..3e179019ca7c 100644
>>> --- a/drivers/nvme/target/nvmet.h
>>> +++ b/drivers/nvme/target/nvmet.h
>>> @@ -184,6 +184,11 @@ static inline u8
>>> nvmet_port_disc_addr_treq_secure_channel(struct nvmet_port *por
>>> return (port->disc_addr.treq & NVME_TREQ_SECURE_CHANNEL_MASK);
>>> }
>>> +static inline bool nvmet_port_secure_channel_required(struct
>>> nvmet_port *port)
>>> +{
>>> + return nvmet_port_disc_addr_treq_secure_channel(port) ==
>>> NVMF_TREQ_REQUIRED;
>>> +}
>>> +
>>> struct nvmet_ctrl {
>>> struct nvmet_subsys *subsys;
>>> struct nvmet_sq **sqs;
>>> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
>>> index 67fffa2e1e4a..5c1518a8bded 100644
>>> --- a/drivers/nvme/target/tcp.c
>>> +++ b/drivers/nvme/target/tcp.c
>>> @@ -1730,6 +1730,54 @@ static int nvmet_tcp_set_queue_sock(struct
>>> nvmet_tcp_queue *queue)
>>> }
>>> #ifdef CONFIG_NVME_TARGET_TCP_TLS
>>
>> You need a stub for the other side of the ifdef?
>
> No. The new function is completely encapsulated by
> CONFIG_NVME_TARGET_TCP_TLS, so no need to add a stub.
Ok.
More information about the Linux-nvme
mailing list