Re: [PATCH 4/4] nvme: check that EUI/GUID/UUID are globally unique

Alan Adamson
2022-06-21T20:40:17Z

> On Jun 21, 2022, at 12:11 PM, Keith Busch <kbusch@kernel.org> wrote:
>
> On Tue, Jun 21, 2022 at 06:40:49PM +0000, Alan Adamson wrote:
>> +static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
>> +{
>> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
>> + void *data;
>> + struct nvme_ns_id_desc *cur;
>> + u16 status = NVME_SC_SUCCESS;
>> + u8 csi;
>> + int pos, len;
>> + bool csi_seen;
>> +
>> + if (!ctrl->subsys->clear_ids)
>> + return status;
>> +
>> + data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
>> + if (!data)
>> + return NVME_SC_INTERNAL;
>> +
>> + status = nvmet_copy_from_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
>> + if (status)
>> + goto out_free;
>> +
>> + for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
>> + cur = data + pos;
>> +
>> + if (cur->nidl == 0)
>> + break;
>> + len = cur->nidl;
>> + if (cur->nidt == NVME_NIDT_CSI) {
>> + memcpy(&csi, data + pos + sizeof(struct nvme_ns_id_desc), NVME_NIDT_CSI_LEN);
>> + csi_seen = 1;
>> + break;
>> + }
>> + len += sizeof(struct nvme_ns_id_desc);
>> + }
>> + if (csi_seen) {
>> + cur = data;
>> + cur->nidt = NVME_NIDT_CSI;
>> + cur->nidl = NVME_NIDT_CSI_LEN;
>> + memcpy(data + sizeof(struct nvme_ns_id_desc), &csi, NVME_NIDT_CSI_LEN);
>> +
>> + cur = data + sizeof(struct nvme_ns_id_desc) + NVME_NIDT_CSI_LEN;
>> + cur->nidt = 0;
>> + cur->nidl = 0;
>> + status = nvmet_copy_to_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
>> + }
>
> This is clearing the other descriptors only if the controller also reports a
> CSI field. I think just do something like the following on top of your patch,
> and should be good to go.
>
> ---
> diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
> index f863cd459652..f9599b0cd129 100644
> --- a/drivers/nvme/target/passthru.c
> +++ b/drivers/nvme/target/passthru.c
> @@ -54,17 +54,15 @@ static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
> }
> len += sizeof(struct nvme_ns_id_desc);
> }
> +
> + memset(data, 0, NVME_IDENTIFY_DATA_SIZE);
> if (csi_seen) {
> cur = data;
> cur->nidt = NVME_NIDT_CSI;
> cur->nidl = NVME_NIDT_CSI_LEN;
> memcpy(data + sizeof(struct nvme_ns_id_desc), &csi, NVME_NIDT_CSI_LEN);
> -
> - cur = data + sizeof(struct nvme_ns_id_desc) + NVME_NIDT_CSI_LEN;
> - cur->nidt = 0;
> - cur->nidl = 0;
> - status = nvmet_copy_to_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
> }
> + status = nvmet_copy_to_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
> out_free:
> kfree(data);
> return status;
> --

Yes I can do that. Originally I was doing that, but wanted to avoid the 4K copy of zeros.

Thanks,

Alan