mi: Allow Admin-message sized More Processing Required responses

Devices may implement their MPR response as an actual Admin response
message, rather than the simple MI-only message described in 4.1.2.3 of
NVMe-MI v1.2b.

Allow this, but with some fairly stringent header checks. Add a test for
this behaviour too.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

diff --git a/src/nvme/mi-mctp.c b/src/nvme/mi-mctp.c
index ae604f22b0c930df7cec49c0a2a1454ed68ecdb2..1f36722047a133211aa36d0e42d4ba63d6cff682 100644 (file)
--- a/src/nvme/mi-mctp.c
+++ b/src/nvme/mi-mctp.c
@@ -179,13 +179,15 @@ struct nvme_mi_msg_resp_mpr {
  * populate the worst-case expected processing time, given in milliseconds.
  */
 static bool nvme_mi_mctp_resp_is_mpr(struct nvme_mi_resp *resp, size_t len,
-                                    unsigned int *mpr_time)
+                                    __le32 mic, unsigned int *mpr_time)
 {
+       struct nvme_mi_admin_resp_hdr *admin_msg;
        struct nvme_mi_msg_resp_mpr *msg;
-       __le32 mic;
+       size_t clen;
        __u32 crc;
 
-       if (len != sizeof(*msg) + sizeof(mic))
+       /* We need at least the minimal header plus checksum */
+       if (len < sizeof(*msg) + sizeof(mic))
                return false;
 
        msg = (struct nvme_mi_msg_resp_mpr *)resp->hdr;
@@ -193,22 +195,42 @@ static bool nvme_mi_mctp_resp_is_mpr(struct nvme_mi_resp *resp, size_t len,
        if (msg->status != NVME_MI_RESP_MPR)
                return false;
 
-       /* We can't use verify_resp_mic here, as the response structure has
-        * not been laid-out properly in resp yet (this is deferred until
-        * we have the actual response).
+       /* Find and verify the MIC from the response, which may not be laid out
+        * in resp as we expect. We have to preserve resp->hdr_len and
+        * resp->data_len, as we will need them for the eventual reply message.
+        * Because of that, we can't use verify_resp_mic here.
         *
-        * We know the data is a fixed size, and linear in the hdr buf, so
-        * calculation is fairly simple. We do need to find the MIC data
-        * though, which could either be in the header buf (if the original
-        * header was larger than the minimal header message), or the start of
-        * the data buf (otherwise).
+        * If the packet was at the expected response size, then mic will
+        * be set already; if not, find it within the header/data buffers.
+        */
+
+       /* Devices may send a MPR response as a full-sized Admin response,
+        * rather than the minimal MI-only header. Allow this, but only if the
+        * type indicates admin, and the allocated response header is the
+        * correct size for an Admin response.
+        */
+       if (((msg->hdr.nmp >> 3) & 0xf) == NVME_MI_MT_ADMIN &&
+           len == sizeof(*admin_msg) + sizeof(mic) &&
+           resp->hdr_len == sizeof(*admin_msg)) {
+               if (resp->data_len)
+                       mic = *(__le32 *)resp->data;
+       } else if (len == sizeof(*msg) + sizeof(mic)) {
+               if (resp->hdr_len > sizeof(*msg))
+                       mic = *(__le32 *)(msg + 1);
+               else if (resp->data_len)
+                       mic = *(__le32 *)(resp->data);
+       } else {
+               return false;
+       }
+
+       /* Since our response is just a header, we're guaranteed to have
+        * all data in resp->hdr. The response may be shorter than the expected
+        * header though, so clamp to len.
         */
-       if (resp->hdr_len > sizeof(*msg))
-               mic = *(__le32 *)(msg + 1);
-       else
-               mic = *(__le32 *)(resp->data);
+       len -= sizeof(mic);
+       clen = len < resp->hdr_len ? len : resp->hdr_len;
 
-       crc = ~nvme_mi_crc32_update(0xffffffff, msg, sizeof(*msg));
+       crc = ~nvme_mi_crc32_update(0xffffffff, resp->hdr, clen);
        if (le32_to_cpu(mic) != crc)
                return false;
 
@@ -369,7 +391,7 @@ retry:
         * header fields. However, we need to do this in the transport in order
         * to keep the tag allocated and retry the recvmsg
         */
-       if (nvme_mi_mctp_resp_is_mpr(resp, len, &mpr_time)) {
+       if (nvme_mi_mctp_resp_is_mpr(resp, len, mic, &mpr_time)) {
                nvme_msg(ep->root, LOG_DEBUG,
                         "Received More Processing Required, waiting for response\n");
 

diff --git a/test/mi-mctp.c b/test/mi-mctp.c
index 5b1a1544628bad4012111f1adf50765d2487399b..0773d000bb60245184c461d38ee3dfbccca80c75 100644 (file)
--- a/test/mi-mctp.c
+++ b/test/mi-mctp.c
@@ -407,6 +407,7 @@ static void test_poll_timeout(nvme_mi_ep_t ep, struct test_peer *peer)
 /* test: send a More Processing Required response, then the actual response */
 struct mpr_tx_info {
        int msg_no;
+       bool admin_quirk;
        size_t final_len;
 };
 
@@ -422,6 +423,9 @@ static int tx_mpr(struct test_peer *peer, void *buf, size_t len)
        case 1:
                peer->tx_buf[4] = NVME_MI_RESP_MPR;
                peer->tx_buf_len = 8;
+               if (tx_info->admin_quirk) {
+                       peer->tx_buf_len = 20;
+               }
                break;
        case 2:
                peer->tx_buf[4] = NVME_MI_RESP_SUCCESS;
@@ -446,6 +450,7 @@ static void test_mpr_mi(nvme_mi_ep_t ep, struct test_peer *peer)
 
        tx_info.msg_no = 1;
        tx_info.final_len = sizeof(struct nvme_mi_mi_resp_hdr) + sizeof(ss_info);
+       tx_info.admin_quirk = false;
 
        peer->tx_fn = tx_mpr;
        peer->tx_data = &tx_info;
@@ -463,6 +468,32 @@ static void test_mpr_admin(nvme_mi_ep_t ep, struct test_peer *peer)
 
        tx_info.msg_no = 1;
        tx_info.final_len = sizeof(struct nvme_mi_admin_resp_hdr) + sizeof(id);
+       tx_info.admin_quirk = false;
+
+       peer->tx_fn = tx_mpr;
+       peer->tx_data = &tx_info;
+
+       ctrl = nvme_mi_init_ctrl(ep, 1);
+
+       rc = nvme_mi_admin_identify_ctrl(ctrl, &id);
+       assert(rc == 0);
+
+       nvme_mi_close_ctrl(ctrl);
+}
+
+/* We have seen drives that send a MPR response as a full Admin message,
+ * rather than a MI message; these have a larger message body
+ */
+static void test_mpr_admin_quirked(nvme_mi_ep_t ep, struct test_peer *peer)
+{
+       struct mpr_tx_info tx_info;
+       struct nvme_id_ctrl id;
+       nvme_mi_ctrl_t ctrl;
+       int rc;
+
+       tx_info.msg_no = 1;
+       tx_info.final_len = sizeof(struct nvme_mi_admin_resp_hdr) + sizeof(id);
+       tx_info.admin_quirk = true;
 
        peer->tx_fn = tx_mpr;
        peer->tx_data = &tx_info;
@@ -638,6 +669,7 @@ struct test {
        DEFINE_TEST(poll_timeout),
        DEFINE_TEST(mpr_mi),
        DEFINE_TEST(mpr_admin),
+       DEFINE_TEST(mpr_admin_quirked),
        DEFINE_TEST(mpr_timeouts),
        DEFINE_TEST(mpr_timeout_clamp),
        DEFINE_TEST(mpr_mprt_zero),