mi: allow non-4-byte-aligned responses

We currently assume that a MI response will be a multiple of four bytes
in length. However, this may not be the case: for example, a Read MI
Data (Controller List) with an even number of controllers, and with an
unpadded response, may only be aligned on a two-byte boundary.

The NVMe-MI spec states, for the MIC field:

    This field is byte aligned.

So, relax the requirement for alignment on the response sizes, and the
expected response size values. We only do this for the mi commands; the
Admin commands still require an aligned value for DLEN.

In doing so, drop the explicit alignment tests, and add a couple that
check that the Controller List example above will work.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Reported-by: Klaus Jensen <its@irrelevant.dk>

diff --git a/src/nvme/mi.c b/src/nvme/mi.c
index 2d780674e96cc8b5d1843ff7de060a34252d19dd..95f5224eb69571e83764516aace6e7e17fcb575d 100644 (file)
--- a/src/nvme/mi.c
+++ b/src/nvme/mi.c
@@ -413,11 +413,6 @@ int nvme_mi_submit(nvme_mi_ep_t ep, struct nvme_mi_req *req,
                return -1;
        }
 
-       if (resp->data_len & 0x3) {
-               errno = EINVAL;
-               return -1;
-       }
-
        if (ep->transport->mic_enabled)
                nvme_mi_calc_req_mic(req);
 

diff --git a/test/mi-mctp.c b/test/mi-mctp.c
index 6d83d427e32af471b7ae1bbafba9c7bd4b7a7f43..1ba2ae8a7a65647d0ace8d6de9c08f31b4757108 100644 (file)
--- a/test/mi-mctp.c
+++ b/test/mi-mctp.c
@@ -293,6 +293,89 @@ static void test_mi_resp_err(nvme_mi_ep_t ep, struct test_peer *peer)
        assert(rc == 0x2);
 }
 
+static void setup_unaligned_ctrl_list_resp(struct test_peer *peer)
+{
+       /* even number of controllers */
+       peer->tx_buf[8] = 0x02;
+       peer->tx_buf[9] = 0x00;
+
+       /* controller ID 1 */
+       peer->tx_buf[10] = 0x01;
+       peer->tx_buf[11] = 0x00;
+
+       /* controller ID 2 */
+       peer->tx_buf[12] = 0x02;
+       peer->tx_buf[13] = 0x00;
+
+       peer->tx_buf_len = 14;
+}
+
+/* Will call through the xfer/submit API expecting a full-sized list (so
+ * resp->data_len is set to sizeof(list)), but the endpoint will return an
+ * unaligned short list.
+ */
+static void test_mi_resp_unaligned(nvme_mi_ep_t ep, struct test_peer *peer)
+{
+       struct nvme_ctrl_list list;
+       int rc;
+
+       setup_unaligned_ctrl_list_resp(peer);
+
+       memset(&list, 0, sizeof(list));
+
+       rc = nvme_mi_mi_read_mi_data_ctrl_list(ep, 0, &list);
+       assert(rc == 0);
+
+       assert(le16_to_cpu(list.num) == 2);
+       assert(le16_to_cpu(list.identifier[0]) == 1);
+       assert(le16_to_cpu(list.identifier[1]) == 2);
+}
+
+/* Will call through the xfer/submit API expecting an unaligned list,
+ * and get a response of exactly that size.
+ */
+static void test_mi_resp_unaligned_expected(nvme_mi_ep_t ep,
+                                           struct test_peer *peer)
+{
+       /* direct access to the raw submit() API */
+       extern int nvme_mi_submit(nvme_mi_ep_t ep, struct nvme_mi_req *req,
+                  struct nvme_mi_resp *resp);
+       struct nvme_mi_mi_resp_hdr resp_hdr;
+       struct nvme_mi_mi_req_hdr req_hdr;
+       struct nvme_ctrl_list list;
+       struct nvme_mi_resp resp;
+       struct nvme_mi_req req;
+       int rc;
+
+       setup_unaligned_ctrl_list_resp(peer);
+
+       memset(&list, 0, sizeof(list));
+
+       memset(&req_hdr, 0, sizeof(req_hdr));
+       req_hdr.hdr.type = NVME_MI_MSGTYPE_NVME;
+       req_hdr.hdr.nmp = (NVME_MI_ROR_REQ << 7) | (NVME_MI_MT_MI << 3);
+       req_hdr.opcode = nvme_mi_mi_opcode_mi_data_read;
+       req_hdr.cdw0 = cpu_to_le32(nvme_mi_dtyp_ctrl_list << 24);
+
+       memset(&req, 0, sizeof(req));
+       req.hdr = &req_hdr.hdr;
+       req.hdr_len = sizeof(req_hdr);
+
+       memset(&resp, 0, sizeof(resp));
+       resp.hdr = &resp_hdr.hdr;
+       resp.hdr_len = sizeof(resp_hdr);
+       resp.data = &list;
+       resp.data_len = peer->tx_buf_len;
+
+       rc = nvme_mi_submit(ep, &req, &resp);
+       assert(rc == 0);
+       assert(resp.data_len == 6); /* 2-byte length, 2*2 byte controller IDs */
+
+       assert(le16_to_cpu(list.num) == 2);
+       assert(le16_to_cpu(list.identifier[0]) == 1);
+       assert(le16_to_cpu(list.identifier[1]) == 2);
+}
+
 static void test_admin_resp_err(nvme_mi_ep_t ep, struct test_peer *peer)
 {
        struct nvme_id_ctrl id;
@@ -340,30 +423,6 @@ static void test_admin_resp_sizes(nvme_mi_ep_t ep, struct test_peer *peer)
        nvme_mi_close_ctrl(ctrl);
 }
 
-/* test: unaligned response sizes - should always report a transport error */
-static void test_admin_resp_sizes_unaligned(nvme_mi_ep_t ep, struct test_peer *peer)
-{
-       struct nvme_id_ctrl id;
-       nvme_mi_ctrl_t ctrl;
-       unsigned int i;
-       int rc;
-
-       ctrl = nvme_mi_init_ctrl(ep, 1);
-       assert(ctrl);
-
-       peer->tx_buf[4] = 0x02; /* internal error */
-
-       for (i = 8; i <= 4096 + 8; i++) {
-               peer->tx_buf_len = i;
-               if (!(i & 0x3))
-                       continue;
-               rc = nvme_mi_admin_identify_ctrl(ctrl, &id);
-               assert(rc < 0);
-       }
-
-       nvme_mi_close_ctrl(ctrl);
-}
-
 /* test: timeout value passed to poll */
 static int poll_fn_timeout_value(struct test_peer *peer, struct pollfd *fds,
                                 nfds_t nfds, int timeout)
@@ -664,9 +723,10 @@ struct test {
        DEFINE_TEST(read_mi_data),
        DEFINE_TEST(poll_err),
        DEFINE_TEST(mi_resp_err),
+       DEFINE_TEST(mi_resp_unaligned),
+       DEFINE_TEST(mi_resp_unaligned_expected),
        DEFINE_TEST(admin_resp_err),
        DEFINE_TEST(admin_resp_sizes),
-       DEFINE_TEST(admin_resp_sizes_unaligned),
        DEFINE_TEST(poll_timeout_value),
        DEFINE_TEST(poll_timeout),
        DEFINE_TEST(mpr_mi),