Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/91238/?format=api
{ "id": 91238, "url": "https://patches.dpdk.org/api/patches/91238/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/1618314611-47978-3-git-send-email-humin29@huawei.com/", "project": { "id": 1, "url": "https://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<1618314611-47978-3-git-send-email-humin29@huawei.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1618314611-47978-3-git-send-email-humin29@huawei.com", "date": "2021-04-13T11:50:01", "name": "[02/12] net/hns3: fix possible mismatches response of mailbox", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "0e71083a360ef5cf381bb64749961d190a15018f", "submitter": { "id": 1944, "url": "https://patches.dpdk.org/api/people/1944/?format=api", "name": "humin (Q)", "email": "humin29@huawei.com" }, "delegate": { "id": 319, "url": "https://patches.dpdk.org/api/users/319/?format=api", "username": "fyigit", "first_name": "Ferruh", "last_name": "Yigit", "email": "ferruh.yigit@amd.com" }, "mbox": "https://patches.dpdk.org/project/dpdk/patch/1618314611-47978-3-git-send-email-humin29@huawei.com/mbox/", "series": [ { "id": 16331, "url": "https://patches.dpdk.org/api/series/16331/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/list/?series=16331", "date": "2021-04-13T11:50:10", "name": "Bugfix for hns3 PMD", "version": 1, "mbox": "https://patches.dpdk.org/series/16331/mbox/" } ], "comments": "https://patches.dpdk.org/api/patches/91238/comments/", "check": "success", "checks": "https://patches.dpdk.org/api/patches/91238/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 31441A0524;\n\tTue, 13 Apr 2021 13:50:42 +0200 (CEST)", "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id 09362160EB7;\n\tTue, 13 Apr 2021 13:50:11 +0200 (CEST)", "from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35])\n by mails.dpdk.org (Postfix) with ESMTP id 5C1ED160E94\n for <dev@dpdk.org>; Tue, 13 Apr 2021 13:50:04 +0200 (CEST)", "from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.59])\n by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4FKP3z2jMYz9ydV\n for <dev@dpdk.org>; Tue, 13 Apr 2021 19:47:43 +0800 (CST)", "from localhost.localdomain (10.69.192.56) by\n DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server id\n 14.3.498.0; Tue, 13 Apr 2021 19:49:55 +0800" ], "From": "\"Min Hu (Connor)\" <humin29@huawei.com>", "To": "<dev@dpdk.org>", "CC": "<ferruh.yigit@intel.com>", "Date": "Tue, 13 Apr 2021 19:50:01 +0800", "Message-ID": "<1618314611-47978-3-git-send-email-humin29@huawei.com>", "X-Mailer": "git-send-email 2.7.4", "In-Reply-To": "<1618314611-47978-1-git-send-email-humin29@huawei.com>", "References": "<1618314611-47978-1-git-send-email-humin29@huawei.com>", "MIME-Version": "1.0", "Content-Type": "text/plain", "X-Originating-IP": "[10.69.192.56]", "X-CFilter-Loop": "Reflected", "Subject": "[dpdk-dev] [PATCH 02/12] net/hns3: fix possible mismatches response\n of mailbox", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "From: Chengwen Feng <fengchengwen@huawei.com>\n\nCurrently, the mailbox synchronous communication between VF and PF use\nthe following fields to maintain communication:\n1. Req_msg_data which was combined by message code and subcode, used\nto match request and response.\n2. Head which means the number of requests successfully sent by VF.\n3. Tail which means the number of responses successfully received by\nVF.\n4. Lost which means the number of requests which are timeout.\n\nThere may possible mismatches of the following situation:\n1. VF sends message A with code=1 subcode=1.\n\tThen head=1, tail=0, lost=0.\n2. PF was blocked about 500ms when processing the message A.\n3. VF will detect message A timeout because it can't get the response\nwithin 500ms.\n\tThen head=1, tail=0, lost=1.\n4. VF sends message B with code=1 subcode=1 which equal message A.\n\tThen head=2, tail=0, lost=1.\n5. PF processes the first message A and send the response message to\nVF.\n6. VF will update tail field to 1, but the lost field will remain\nunchanged because the code/subcode equal message B's, so driver will\nreturn success because now the head(2) equals tail(1) plus lost(1).\nThis will lead to mismatch of request and response.\n\nTo fix the above bug, we use the following scheme:\n1. The message sent from VF was labelled with match_id which was a\nunique 16-bit non-zero value.\n2. The response sent from PF will label with match_id which got from\nthe request.\n3. The VF uses the match_id to match request and response message.\n\nThis scheme depends on the PF driver, if the PF driver don't support\nthen VF will uses the original scheme.\n\nFixes: 463e748964f5 (\"net/hns3: support mailbox\")\nCc: stable@dpdk.org\n\nSigned-off-by: Chengwen Feng <fengchengwen@huawei.com>\nSigned-off-by: Min Hu (Connor) <humin29@huawei.com>\n---\n drivers/net/hns3/hns3_mbx.c | 120 +++++++++++++++++++++++++++++++++++---------\n drivers/net/hns3/hns3_mbx.h | 20 +++++++-\n 2 files changed, 114 insertions(+), 26 deletions(-)", "diff": "diff --git a/drivers/net/hns3/hns3_mbx.c b/drivers/net/hns3/hns3_mbx.c\nindex 4326c7b..8e38649 100644\n--- a/drivers/net/hns3/hns3_mbx.c\n+++ b/drivers/net/hns3/hns3_mbx.c\n@@ -40,14 +40,32 @@ hns3_resp_to_errno(uint16_t resp_code)\n \treturn -EIO;\n }\n \n+static void\n+hns3_mbx_proc_timeout(struct hns3_hw *hw, uint16_t code, uint16_t subcode)\n+{\n+\tif (hw->mbx_resp.matching_scheme ==\n+\t HNS3_MBX_RESP_MATCHING_SCHEME_OF_ORIGINAL) {\n+\t\thw->mbx_resp.lost++;\n+\t\thns3_err(hw,\n+\t\t\t \"VF could not get mbx(%u,%u) head(%u) tail(%u) \"\n+\t\t\t \"lost(%u) from PF\",\n+\t\t\t code, subcode, hw->mbx_resp.head, hw->mbx_resp.tail,\n+\t\t\t hw->mbx_resp.lost);\n+\t\treturn;\n+\t}\n+\n+\thns3_err(hw, \"VF could not get mbx(%u,%u) from PF\", code, subcode);\n+}\n+\n static int\n-hns3_get_mbx_resp(struct hns3_hw *hw, uint16_t code0, uint16_t code1,\n+hns3_get_mbx_resp(struct hns3_hw *hw, uint16_t code, uint16_t subcode,\n \t\t uint8_t *resp_data, uint16_t resp_len)\n {\n #define HNS3_MAX_RETRY_MS\t500\n #define HNS3_WAIT_RESP_US\t100\n \tstruct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);\n \tstruct hns3_mbx_resp_status *mbx_resp;\n+\tbool received;\n \tuint64_t now;\n \tuint64_t end;\n \n@@ -59,8 +77,7 @@ hns3_get_mbx_resp(struct hns3_hw *hw, uint16_t code0, uint16_t code1,\n \n \tnow = get_timeofday_ms();\n \tend = now + HNS3_MAX_RETRY_MS;\n-\twhile ((hw->mbx_resp.head != hw->mbx_resp.tail + hw->mbx_resp.lost) &&\n-\t (now < end)) {\n+\twhile (now < end) {\n \t\tif (__atomic_load_n(&hw->reset.disable_cmd, __ATOMIC_RELAXED)) {\n \t\t\thns3_err(hw, \"Don't wait for mbx respone because of \"\n \t\t\t\t \"disable_cmd\");\n@@ -77,16 +94,20 @@ hns3_get_mbx_resp(struct hns3_hw *hw, uint16_t code0, uint16_t code1,\n \t\thns3_dev_handle_mbx_msg(hw);\n \t\trte_delay_us(HNS3_WAIT_RESP_US);\n \n+\t\tif (hw->mbx_resp.matching_scheme ==\n+\t\t HNS3_MBX_RESP_MATCHING_SCHEME_OF_ORIGINAL)\n+\t\t\treceived = (hw->mbx_resp.head ==\n+\t\t\t\t hw->mbx_resp.tail + hw->mbx_resp.lost);\n+\t\telse\n+\t\t\treceived = hw->mbx_resp.received_match_resp;\n+\t\tif (received)\n+\t\t\tbreak;\n+\n \t\tnow = get_timeofday_ms();\n \t}\n \thw->mbx_resp.req_msg_data = 0;\n \tif (now >= end) {\n-\t\thw->mbx_resp.lost++;\n-\t\thns3_err(hw,\n-\t\t\t \"VF could not get mbx(%u,%u) head(%u) tail(%u) \"\n-\t\t\t \"lost(%u) from PF\",\n-\t\t\t code0, code1, hw->mbx_resp.head, hw->mbx_resp.tail,\n-\t\t\t hw->mbx_resp.lost);\n+\t\thns3_mbx_proc_timeout(hw, code, subcode);\n \t\treturn -ETIME;\n \t}\n \trte_io_rmb();\n@@ -101,6 +122,29 @@ hns3_get_mbx_resp(struct hns3_hw *hw, uint16_t code0, uint16_t code1,\n \treturn 0;\n }\n \n+static void\n+hns3_mbx_prepare_resp(struct hns3_hw *hw, uint16_t code, uint16_t subcode)\n+{\n+\t/*\n+\t * Init both matching scheme fields because we may not know the exact\n+\t * scheme will be used when in the initial phase.\n+\t *\n+\t * Also, there are OK to init both matching scheme fields even though\n+\t * we get the exact scheme which is used.\n+\t */\n+\thw->mbx_resp.req_msg_data = (uint32_t)code << 16 | subcode;\n+\thw->mbx_resp.head++;\n+\n+\t/* Update match_id and ensure the value of match_id is not zero */\n+\thw->mbx_resp.match_id++;\n+\tif (hw->mbx_resp.match_id == 0)\n+\t\thw->mbx_resp.match_id = 1;\n+\thw->mbx_resp.received_match_resp = false;\n+\n+\thw->mbx_resp.resp_status = 0;\n+\tmemset(hw->mbx_resp.additional_info, 0, HNS3_MBX_MAX_RESP_DATA_SIZE);\n+}\n+\n int\n hns3_send_mbx_msg(struct hns3_hw *hw, uint16_t code, uint16_t subcode,\n \t\t const uint8_t *msg_data, uint8_t msg_len, bool need_resp,\n@@ -138,8 +182,8 @@ hns3_send_mbx_msg(struct hns3_hw *hw, uint16_t code, uint16_t subcode,\n \tif (need_resp) {\n \t\treq->mbx_need_resp |= HNS3_MBX_NEED_RESP_BIT;\n \t\trte_spinlock_lock(&hw->mbx_resp.lock);\n-\t\thw->mbx_resp.req_msg_data = (uint32_t)code << 16 | subcode;\n-\t\thw->mbx_resp.head++;\n+\t\thns3_mbx_prepare_resp(hw, code, subcode);\n+\t\treq->match_id = hw->mbx_resp.match_id;\n \t\tret = hns3_cmd_send(hw, &desc, 1);\n \t\tif (ret) {\n \t\t\thw->mbx_resp.head--;\n@@ -244,6 +288,46 @@ hns3_update_resp_position(struct hns3_hw *hw, uint32_t resp_msg)\n }\n \n static void\n+hns3_handle_mbx_response(struct hns3_hw *hw, struct hns3_mbx_pf_to_vf_cmd *req)\n+{\n+\tstruct hns3_mbx_resp_status *resp = &hw->mbx_resp;\n+\tuint32_t msg_data;\n+\n+\tif (req->match_id != 0) {\n+\t\t/*\n+\t\t * If match_id is not zero, it means PF support copy request's\n+\t\t * match_id to its response. So VF could use the match_id\n+\t\t * to match the request.\n+\t\t */\n+\t\tif (resp->matching_scheme !=\n+\t\t HNS3_MBX_RESP_MATCHING_SCHEME_OF_MATCH_ID) {\n+\t\t\tresp->matching_scheme =\n+\t\t\t\tHNS3_MBX_RESP_MATCHING_SCHEME_OF_MATCH_ID;\n+\t\t\thns3_info(hw, \"detect mailbox support match id!\");\n+\t\t}\n+\t\tif (req->match_id == resp->match_id) {\n+\t\t\tresp->resp_status = hns3_resp_to_errno(req->msg[3]);\n+\t\t\tmemcpy(resp->additional_info, &req->msg[4],\n+\t\t\t HNS3_MBX_MAX_RESP_DATA_SIZE);\n+\t\t\trte_io_wmb();\n+\t\t\tresp->received_match_resp = true;\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\t/*\n+\t * If the below instructions can be executed, it means PF does not\n+\t * support copy request's match_id to its response. So VF follows the\n+\t * original scheme to process.\n+\t */\n+\tresp->resp_status = hns3_resp_to_errno(req->msg[3]);\n+\tmemcpy(resp->additional_info, &req->msg[4],\n+\t HNS3_MBX_MAX_RESP_DATA_SIZE);\n+\tmsg_data = (uint32_t)req->msg[1] << 16 | req->msg[2];\n+\thns3_update_resp_position(hw, msg_data);\n+}\n+\n+static void\n hns3_link_fail_parse(struct hns3_hw *hw, uint8_t link_fail_code)\n {\n \tswitch (link_fail_code) {\n@@ -319,15 +403,11 @@ hns3_handle_promisc_info(struct hns3_hw *hw, uint16_t promisc_en)\n void\n hns3_dev_handle_mbx_msg(struct hns3_hw *hw)\n {\n-\tstruct hns3_mbx_resp_status *resp = &hw->mbx_resp;\n \tstruct hns3_cmq_ring *crq = &hw->cmq.crq;\n \tstruct hns3_mbx_pf_to_vf_cmd *req;\n \tstruct hns3_cmd_desc *desc;\n-\tuint32_t msg_data;\n \tuint8_t opcode;\n \tuint16_t flag;\n-\tuint8_t *temp;\n-\tint i;\n \n \trte_spinlock_lock(&hw->cmq.crq.lock);\n \n@@ -355,15 +435,7 @@ hns3_dev_handle_mbx_msg(struct hns3_hw *hw)\n \n \t\tswitch (opcode) {\n \t\tcase HNS3_MBX_PF_VF_RESP:\n-\t\t\tresp->resp_status = hns3_resp_to_errno(req->msg[3]);\n-\n-\t\t\ttemp = (uint8_t *)&req->msg[4];\n-\t\t\tfor (i = 0; i < HNS3_MBX_MAX_RESP_DATA_SIZE; i++) {\n-\t\t\t\tresp->additional_info[i] = *temp;\n-\t\t\t\ttemp++;\n-\t\t\t}\n-\t\t\tmsg_data = (uint32_t)req->msg[1] << 16 | req->msg[2];\n-\t\t\thns3_update_resp_position(hw, msg_data);\n+\t\t\thns3_handle_mbx_response(hw, req);\n \t\t\tbreak;\n \t\tcase HNS3_MBX_LINK_STAT_CHANGE:\n \t\t\thns3vf_handle_link_change_event(hw, req);\ndiff --git a/drivers/net/hns3/hns3_mbx.h b/drivers/net/hns3/hns3_mbx.h\nindex 45aa4cd..0e9194d 100644\n--- a/drivers/net/hns3/hns3_mbx.h\n+++ b/drivers/net/hns3/hns3_mbx.h\n@@ -83,12 +83,26 @@ enum hns3_mbx_link_fail_subcode {\n #define HNS3_MBX_RING_MAP_BASIC_MSG_NUM\t3\n #define HNS3_MBX_RING_NODE_VARIABLE_NUM\t3\n \n+enum {\n+\tHNS3_MBX_RESP_MATCHING_SCHEME_OF_ORIGINAL = 0,\n+\tHNS3_MBX_RESP_MATCHING_SCHEME_OF_MATCH_ID\n+};\n+\n struct hns3_mbx_resp_status {\n \trte_spinlock_t lock; /* protects against contending sync cmd resp */\n+\n+\tuint8_t matching_scheme;\n+\n+\t/* The following fields used in the matching scheme for original */\n \tuint32_t req_msg_data;\n \tuint32_t head;\n \tuint32_t tail;\n \tuint32_t lost;\n+\n+\t/* The following fields used in the matching scheme for match_id */\n+\tuint16_t match_id;\n+\tbool received_match_resp;\n+\n \tint resp_status;\n \tuint8_t additional_info[HNS3_MBX_MAX_RESP_DATA_SIZE];\n };\n@@ -106,7 +120,8 @@ struct hns3_mbx_vf_to_pf_cmd {\n \tuint8_t mbx_need_resp;\n \tuint8_t rsv1;\n \tuint8_t msg_len;\n-\tuint8_t rsv2[3];\n+\tuint8_t rsv2;\n+\tuint16_t match_id;\n \tuint8_t msg[HNS3_MBX_MAX_MSG_SIZE];\n };\n \n@@ -114,7 +129,8 @@ struct hns3_mbx_pf_to_vf_cmd {\n \tuint8_t dest_vfid;\n \tuint8_t rsv[3];\n \tuint8_t msg_len;\n-\tuint8_t rsv1[3];\n+\tuint8_t rsv1;\n+\tuint16_t match_id;\n \tuint16_t msg[8];\n };\n \n", "prefixes": [ "02/12" ] }