Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/133963/?format=api
{ "id": 133963, "url": "http://patches.dpdk.org/api/patches/133963/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/20231108034434.559030-2-haijie1@huawei.com/", "project": { "id": 1, "url": "http://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": "<20231108034434.559030-2-haijie1@huawei.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20231108034434.559030-2-haijie1@huawei.com", "date": "2023-11-08T03:44:30", "name": "[1/5] net/hns3: fix sync mailbox failure forever", "commit_ref": null, "pull_url": null, "state": "changes-requested", "archived": true, "hash": "b2d31f4486fd1c6f0b7ae433b8ef8f757790ab0d", "submitter": { "id": 2935, "url": "http://patches.dpdk.org/api/people/2935/?format=api", "name": "Jie Hai", "email": "haijie1@huawei.com" }, "delegate": { "id": 319, "url": "http://patches.dpdk.org/api/users/319/?format=api", "username": "fyigit", "first_name": "Ferruh", "last_name": "Yigit", "email": "ferruh.yigit@amd.com" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/20231108034434.559030-2-haijie1@huawei.com/mbox/", "series": [ { "id": 30196, "url": "http://patches.dpdk.org/api/series/30196/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=30196", "date": "2023-11-08T03:44:29", "name": "net/hns3: fix and refactor mailbox code", "version": 1, "mbox": "http://patches.dpdk.org/series/30196/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/133963/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/133963/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 BA764432CE;\n\tWed, 8 Nov 2023 04:48:57 +0100 (CET)", "from mails.dpdk.org (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id CF30142DE2;\n\tWed, 8 Nov 2023 04:48:53 +0100 (CET)", "from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187])\n by mails.dpdk.org (Postfix) with ESMTP id 5548142DC7\n for <dev@dpdk.org>; Wed, 8 Nov 2023 04:48:49 +0100 (CET)", "from kwepemd100004.china.huawei.com (unknown [172.30.72.53])\n by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4SQ9zq226Bzfb1w;\n Wed, 8 Nov 2023 11:48:39 +0800 (CST)", "from localhost.localdomain (10.67.165.2) by\n kwepemd100004.china.huawei.com (7.221.188.31) with Microsoft SMTP Server\n (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id\n 15.2.1258.23; Wed, 8 Nov 2023 11:48:15 +0800" ], "From": "Jie Hai <haijie1@huawei.com>", "To": "<dev@dpdk.org>, Yisen Zhuang <yisen.zhuang@huawei.com>, Huisong Li\n <lihuisong@huawei.com>, \"Min Hu (Connor)\" <humin29@huawei.com>, Chunsong Feng\n <fengchunsong@huawei.com>, \"Wei Hu (Xavier)\" <xavier.huwei@huawei.com>, Hao\n Chen <chenh@yusur.tech>", "Subject": "[PATCH 1/5] net/hns3: fix sync mailbox failure forever", "Date": "Wed, 8 Nov 2023 11:44:30 +0800", "Message-ID": "<20231108034434.559030-2-haijie1@huawei.com>", "X-Mailer": "git-send-email 2.30.0", "In-Reply-To": "<20231108034434.559030-1-haijie1@huawei.com>", "References": "<20231108034434.559030-1-haijie1@huawei.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Content-Type": "text/plain", "X-Originating-IP": "[10.67.165.2]", "X-ClientProxiedBy": "dggems702-chm.china.huawei.com (10.3.19.179) To\n kwepemd100004.china.huawei.com (7.221.188.31)", "X-CFilter-Loop": "Reflected", "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" }, "content": "From: Dengdui Huang <huangdengdui@huawei.com>\n\nCurrently, hns3 VF driver uses the following points to match\nthe response and request message for the mailbox synchronous\nmessage between VF and PF.\n1. req_msg_data which is consist of message code and subcode,\n is used to match request and response.\n2. head means the number of send success for VF.\n3. tail means the number of receive success for VF.\n4. lost means the number of send timeout for VF.\nAnd 'head', 'tail' and 'lost' are dynamically updated during\nthe communication.\n\nNow there is a issue that all sync mailbox message will\nsend failure forever at the flollowing case:\n1. VF sends the message A\n then head=UINT32_MAX-1, tail=UINT32_MAX-3, lost=2.\n2. VF sends the message B\n then head=UINT32_MAX, tail=UINT32_MAX-2, lost=2.\n3. VF sends the message C, the message will be timeout because\n it can't get the response within 500ms.\n then head=0, tail=0, lost=2\n note: tail is assigned to head if tail > head according to\n current code logic. From now on, all subsequent sync milbox\n messages fail to be sent.\n\nIt's very complicated to use the fields 'lost','tail','head'.\nThe code and subcode of the request sync mailbox are used as the\nmatching code of the message, which is used to match the response\nmessage for receiving the synchronization response.\n\nThis patch drops these fields and uses the following solution\nto solve this issue:\nIn the handling response message process, using the req_msg_data\nof the request and response message to judge whether the sync\nmailbox message has been received.\n\nFixes: 463e748964f5 (\"net/hns3: support mailbox\")\nCc: stable@dpdk.org\n\nSigned-off-by: Dengdui Huang <huangdengdui@huawei.com>\nSigned-off-by: Jie Hai <haijie1@huawei.com>\n---\n drivers/net/hns3/hns3_cmd.c | 3 --\n drivers/net/hns3/hns3_mbx.c | 81 ++++++-------------------------------\n drivers/net/hns3/hns3_mbx.h | 10 -----\n 3 files changed, 13 insertions(+), 81 deletions(-)", "diff": "diff --git a/drivers/net/hns3/hns3_cmd.c b/drivers/net/hns3/hns3_cmd.c\nindex a5c4c11dc8c4..2c1664485bef 100644\n--- a/drivers/net/hns3/hns3_cmd.c\n+++ b/drivers/net/hns3/hns3_cmd.c\n@@ -731,9 +731,6 @@ hns3_cmd_init(struct hns3_hw *hw)\n \thw->cmq.csq.next_to_use = 0;\n \thw->cmq.crq.next_to_clean = 0;\n \thw->cmq.crq.next_to_use = 0;\n-\thw->mbx_resp.head = 0;\n-\thw->mbx_resp.tail = 0;\n-\thw->mbx_resp.lost = 0;\n \thns3_cmd_init_regs(hw);\n \n \trte_spinlock_unlock(&hw->cmq.crq.lock);\ndiff --git a/drivers/net/hns3/hns3_mbx.c b/drivers/net/hns3/hns3_mbx.c\nindex 8e0a58aa02d8..f1743c195efa 100644\n--- a/drivers/net/hns3/hns3_mbx.c\n+++ b/drivers/net/hns3/hns3_mbx.c\n@@ -40,23 +40,6 @@ 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 code, uint16_t subcode,\n \t\t uint8_t *resp_data, uint16_t resp_len)\n@@ -67,7 +50,6 @@ hns3_get_mbx_resp(struct hns3_hw *hw, uint16_t code, uint16_t subcode,\n \tstruct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);\n \tstruct hns3_mbx_resp_status *mbx_resp;\n \tuint32_t wait_time = 0;\n-\tbool received;\n \n \tif (resp_len > HNS3_MBX_MAX_RESP_DATA_SIZE) {\n \t\thns3_err(hw, \"VF mbx response len(=%u) exceeds maximum(=%d)\",\n@@ -93,20 +75,14 @@ hns3_get_mbx_resp(struct hns3_hw *hw, uint16_t code, uint16_t subcode,\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\tif (hw->mbx_resp.received_match_resp)\n \t\t\tbreak;\n \n \t\twait_time += HNS3_WAIT_RESP_US;\n \t}\n \thw->mbx_resp.req_msg_data = 0;\n \tif (wait_time >= mbx_time_limit) {\n-\t\thns3_mbx_proc_timeout(hw, code, subcode);\n+\t\thns3_err(hw, \"VF could not get mbx(%u,%u) from PF\", code, subcode);\n \t\treturn -ETIME;\n \t}\n \trte_io_rmb();\n@@ -132,7 +108,6 @@ hns3_mbx_prepare_resp(struct hns3_hw *hw, uint16_t code, uint16_t subcode)\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@@ -185,7 +160,6 @@ hns3_send_mbx_msg(struct hns3_hw *hw, uint16_t code, uint16_t 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 \t\t\trte_spinlock_unlock(&hw->mbx_resp.lock);\n \t\t\thns3_err(hw, \"VF failed(=%d) to send mbx message to PF\",\n \t\t\t\t ret);\n@@ -254,41 +228,10 @@ hns3_handle_asserting_reset(struct hns3_hw *hw,\n \thns3_schedule_reset(HNS3_DEV_HW_TO_ADAPTER(hw));\n }\n \n-/*\n- * Case1: receive response after timeout, req_msg_data\n- * is 0, not equal resp_msg, do lost--\n- * Case2: receive last response during new send_mbx_msg,\n- *\t req_msg_data is different with resp_msg, let\n- *\t lost--, continue to wait for response.\n- */\n-static void\n-hns3_update_resp_position(struct hns3_hw *hw, uint32_t resp_msg)\n-{\n-\tstruct hns3_mbx_resp_status *resp = &hw->mbx_resp;\n-\tuint32_t tail = resp->tail + 1;\n-\n-\tif (tail > resp->head)\n-\t\ttail = resp->head;\n-\tif (resp->req_msg_data != resp_msg) {\n-\t\tif (resp->lost)\n-\t\t\tresp->lost--;\n-\t\thns3_warn(hw, \"Received a mismatched response req_msg(%x) \"\n-\t\t\t \"resp_msg(%x) head(%u) tail(%u) lost(%u)\",\n-\t\t\t resp->req_msg_data, resp_msg, resp->head, tail,\n-\t\t\t resp->lost);\n-\t} else if (tail + resp->lost > resp->head) {\n-\t\tresp->lost--;\n-\t\thns3_warn(hw, \"Received a new response again resp_msg(%x) \"\n-\t\t\t \"head(%u) tail(%u) lost(%u)\", resp_msg,\n-\t\t\t resp->head, tail, resp->lost);\n-\t}\n-\trte_io_wmb();\n-\tresp->tail = tail;\n-}\n-\n static void\n hns3_handle_mbx_response(struct hns3_hw *hw, struct hns3_mbx_pf_to_vf_cmd *req)\n {\n+#define HNS3_MBX_RESP_CODE_OFFSET 16\n \tstruct hns3_mbx_resp_status *resp = &hw->mbx_resp;\n \tuint32_t msg_data;\n \n@@ -298,12 +241,6 @@ hns3_handle_mbx_response(struct hns3_hw *hw, struct hns3_mbx_pf_to_vf_cmd *req)\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@@ -319,11 +256,19 @@ hns3_handle_mbx_response(struct hns3_hw *hw, struct hns3_mbx_pf_to_vf_cmd *req)\n \t * support copy request's match_id to its response. So VF follows the\n \t * original scheme to process.\n \t */\n+\tmsg_data = (uint32_t)req->msg[1] << HNS3_MBX_RESP_CODE_OFFSET | req->msg[2];\n+\tif (resp->req_msg_data != msg_data) {\n+\t\thns3_warn(hw,\n+\t\t\t\"received response tag (%u) is mismatched with requested tag (%u)\",\n+\t\t\tmsg_data, resp->req_msg_data);\n+\t\treturn;\n+\t}\n+\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+\trte_io_wmb();\n+\tresp->received_match_resp = true;\n }\n \n static void\ndiff --git a/drivers/net/hns3/hns3_mbx.h b/drivers/net/hns3/hns3_mbx.h\nindex c378783c6ca4..4a328802b920 100644\n--- a/drivers/net/hns3/hns3_mbx.h\n+++ b/drivers/net/hns3/hns3_mbx.h\n@@ -93,21 +93,11 @@ enum hns3_mbx_link_fail_subcode {\n #define HNS3_MBX_MAX_RESP_DATA_SIZE\t8\n #define HNS3_MBX_DEF_TIME_LIMIT_MS\t500\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", "prefixes": [ "1/5" ] }