[PATCH 18/18] nvmet-tcp: peek icreq before starting TLS
Hannes Reinecke
hare at suse.de
Tue Sep 12 04:48:42 PDT 2023
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.
Cheers,
Hannes
More information about the Linux-nvme
mailing list