On Fri, 2025-08-29 at 22:17 -0600, Keith Busch wrote:
> On Thu, Aug 28, 2025 at 05:25:31PM +1000, Wilfred Mallawa wrote:
> > +static uint16_t nvme_sec_prot_spdm_send(NvmeCtrl *n, NvmeRequest
> > *req)
> > +{
> > + StorageSpdmTransportHeader hdr = {0};
> > + g_autofree uint8_t *sec_buf = NULL;
> > + uint32_t transfer_len = le32_to_cpu(req->cmd.cdw11);
> > + uint32_t transport_transfer_len = transfer_len;
> > + uint32_t dw10 = le32_to_cpu(req->cmd.cdw10);
> > + uint32_t recvd;
> > + uint16_t nvme_cmd_status, ret;
> > + uint8_t secp = extract32(dw10, 24, 8);
> > + uint8_t spsp1 = extract32(dw10, 16, 8);
> > + uint8_t spsp0 = extract32(dw10, 8, 8);
> > + bool spdm_res;
> > +
> > + transport_transfer_len += sizeof(hdr);
> > + if (transport_transfer_len >
> > SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE) {
> > + return NVME_NO_COMPLETE | NVME_DNR;
> > + }
> > +
> > + /* Generate the NVMe transport header */
> > + hdr.security_protocol = secp;
> > + hdr.security_protocol_specific = cpu_to_le16((spsp1 << 8) |
> > spsp0);
> > + hdr.length = cpu_to_le32(transfer_len);
> > +
> > + sec_buf = g_malloc0(transport_transfer_len);
> > + if (!sec_buf) {
> > + return NVME_NO_COMPLETE | NVME_DNR;
> > + }
> > +
> > + /* Attach the transport header */
> > + memcpy(sec_buf, &hdr, sizeof(hdr));
> > + ret = nvme_h2c(n, sec_buf + sizeof(hdr), transfer_len, req);
> > + if (ret) {
> > + return NVME_NO_COMPLETE | NVME_DNR;
>
> The "NO_COMPLETE" is a special -1 value so or'ing it with anything
> doesn't make sense.
>
> But more importantly, what's supposed to complete it? This is
> supposed
> to be used as a return code when you have an async context to
> complete
> it later, and there doesn't appear to be one here.
Okay I totally misinterpreted it's use-case, and yeah there's no async
context here. I will update the error return values for V3. Thanks for
the feedback!
Regards,
Wilfred
