Re: [PATCH RFC 03/11] nvmet: Add nvmet_fabrics_ops flag to indicate SGLs not supported

Damien Le Moal
2025-03-13T09:02:32Z

On 3/13/25 14:18, Mike Christie wrote:
> The nvmet_mdev_pci driver does not initially support SGLs. In some
> prelim testing I don't think there will be a perf gain (the virt related
> interface may be the major bottleneck so I may not notice) so I wasn't
> sure if they will be required/needed. This adds a nvmet_fabrics_ops flag
> so we can tell nvmet core to tell the host we do not supports SGLS.

That is a major spec violation as NVMe fabrics mandates SGL support.
So at the very least, this needs a big fat warning comment explaining what this
is about.

>
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
> drivers/nvme/target/admin-cmd.c | 13 +++++++------
> drivers/nvme/target/nvmet.h | 1 +
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index acc138bbf8f2..486ed6f7b717 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -755,12 +755,13 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
> id->awun = 0;
> id->awupf = 0;
>
> - /* we always support SGLs */
> - id->sgls = cpu_to_le32(NVME_CTRL_SGLS_BYTE_ALIGNED);
> - if (ctrl->ops->flags & NVMF_KEYED_SGLS)
> - id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_KSDBDS);
> - if (req->port->inline_data_size)
> - id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_SAOS);
> + if (!(ctrl->ops->flags & NVMF_SGLS_NOT_SUPP)) {
> + id->sgls = cpu_to_le32(NVME_CTRL_SGLS_BYTE_ALIGNED);
> + if (ctrl->ops->flags & NVMF_KEYED_SGLS)
> + id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_KSDBDS);
> + if (req->port->inline_data_size)
> + id->sgls |= cpu_to_le32(NVME_CTRL_SGLS_SAOS);
> + }
>
> strscpy(id->subnqn, ctrl->subsys->subsysnqn, sizeof(id->subnqn));
>
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index fcf4f460dc9a..ec3d10eb316a 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -403,6 +403,7 @@ struct nvmet_fabrics_ops {
> unsigned int flags;
> #define NVMF_KEYED_SGLS (1 << 0)
> #define NVMF_METADATA_SUPPORTED (1 << 1)
> +#define NVMF_SGLS_NOT_SUPP (1 << 2)
> void (*queue_response)(struct nvmet_req *req);
> int (*add_port)(struct nvmet_port *port);
> void (*remove_port)(struct nvmet_port *port);

--
Damien Le Moal
Western Digital Research