List comments

GET /api/patches/40750/comments/
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

[
    {
        "id": 82325,
        "web_url": "http://patches.dpdk.org/comment/82325/",
        "msgid": "<3146157e-4bc1-0662-45cb-0ae8d1626e6e@intel.com>",
        "list_archive_url": "https://inbox.dpdk.org/dev/3146157e-4bc1-0662-45cb-0ae8d1626e6e@intel.com",
        "date": "2018-06-18T08:51:09",
        "subject": "Re: [dpdk-dev] [PATCH 06/22] ethdev: support attach or detach share\n\tdevice from secondary",
        "submitter": {
            "id": 4,
            "url": "http://patches.dpdk.org/api/people/4/",
            "name": "Burakov, Anatoly",
            "email": "anatoly.burakov@intel.com"
        },
        "content": "On 07-Jun-18 1:38 PM, Qi Zhang wrote:\n> This patch cover the multi-process hotplug case when a share device\n> attach/detach request be issued from secondary process, the implementation\n> references malloc_mp.c.\n> \n> device attach on secondary:\n> a) seconary send asycn request to primary and wait on a condition\n>     which will be released by matched response from primary.\n> b) primary receive the request and attach the new device if failed\n>     goto i).\n> c) primary forward attach request to all secondary as async request\n>     (because this in mp thread context, use sync request will deadlock)\n> d) secondary receive request and attach device and send reply.\n> e) primary check the reply if all success go to j).\n> f) primary send attach rollback async request to all secondary.\n> g) secondary receive the request and detach device and send reply.\n> h) primary receive the reply and detach device as rollback action.\n> i) send fail response to secondary, goto k).\n> j) send success response to secondary.\n> k) secondary process receive response and return.\n> \n> device detach on secondary:\n> a) secondary send async request to primary and wait on a condition\n>     which will be released by matched response from primary.\n> b) primary receive the request and  perform pre-detach check, if device\n>     is locked, goto j).\n> c) primary send pre-detach async request to all secondary.\n> d) secondary perform pre-detach check and send reply.\n> e) primary check the reply if any fail goto j).\n> f) primary send detach async request to all secondary\n> g) secondary detach the device and send reply\n> h) primary detach the device.\n> i) send success response to secondary, goto k).\n> j) send fail response to secondary.\n> k) secondary process receive response and return.\n> \n> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>\n> ---\n\n<snip>\n\n> +TAILQ_HEAD(mp_request_list, mp_request);\n> +static struct {\n> +\tstruct mp_request_list list;\n> +\tpthread_mutex_t lock;\n> +} mp_request_list = {\n> +\t.list = TAILQ_HEAD_INITIALIZER(mp_request_list.list),\n> +\t.lock = PTHREAD_MUTEX_INITIALIZER\n> +};\n> +\n> +#define MP_TIMEOUT_S 5 /**< 5 seconds timeouts */\n\nPatch number 4 should've used this #define to set up its timeout.\n\n> +\n> +static struct mp_request *\n> +find_request_by_id(uint64_t id)\n> +{\n> +\tstruct mp_request *req;\n> +\n> +\tTAILQ_FOREACH(req, &mp_request_list.list, next) {\n> +\t\tif (req->user_req.id == id)\n> +\t\t\tbreak;\n> +\t}\n> +\treturn req;\n> +}\n> +\n\n<snip>\n\n> +\t\tif (resp->result)\n> +\t\t\treturn resp->result;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int\n> +send_response_to_secondary(const struct eth_dev_mp_req *req, int result)\n> +{\n> +\tstruct rte_mp_msg resp_msg = {0};\n\nI've been bitten by this in the past - some compilers (*cough* clang \n*cough*) don't like this kind of zero-initialization depending on which \ntype of parameter comes first in the structure, so i would refrain from \nusing it and used memset(0) instead.\n\n> +\tstruct eth_dev_mp_req *resp =\n> +\t\t(struct eth_dev_mp_req *)resp_msg.param;\n> +\tint ret = 0;\n> +\n> +\tresp_msg.len_param = sizeof(*resp);\n> +\tstrcpy(resp_msg.name, ETH_DEV_MP_ACTION_RESPONSE);\n\nhere and in other places - strlcpy()?\n\n> +\tmemcpy(resp, req, sizeof(*req));\n> +\tresp->result = result;\n> +\n> +\tret = rte_mp_sendmsg(&resp_msg);\n> +\tif (ret)\n> +\t\tethdev_log(ERR, \"failed to send response to secondary\\n\");\n> +\n> +\treturn ret;\n> +}\n> +\n> +static int\n> +handle_async_attach_response(const struct rte_mp_msg *request,\n> +\t\t\t     const struct rte_mp_reply *reply)\n> +{\n\n<snip>\n\n> +\telse\n> +\t\treturn -1;\n> +\tdo {\n> +\t\tret = rte_mp_request_async(&mp_req, &ts, clb);\n> +\t} while (ret != 0 && rte_errno == EEXIST);\n> +\n> +\tif (ret)\n> +\t\tethdev_log(ERR, \"couldn't send async request\\n\");\n> +\tentry = find_request_by_id(req->id > +\t(void)entry;\n\nWhy did you look up entry and then marked it as used without checking \nthe return value? Leftover? Some code missing?\n\n> +\treturn ret;\n> +}\n> +\n> +static int handle_secondary_request(const struct rte_mp_msg *msg,\n> +\t\t\t\t    const void *peer __rte_unused)\n> +{\n> +\tconst struct eth_dev_mp_req *req =\n> +\t\t(const struct eth_dev_mp_req *)msg->param;\n> +\tstruct eth_dev_mp_req tmp_req;\n\n<snip>\n\n> @@ -124,10 +490,101 @@ static int handle_primary_request(const struct rte_mp_msg *msg, const void *peer\n>   \treturn 0;\n>   }\n>   \n> +/**\n> + * secondary to primary request.\n> + *\n> + * device attach:\n> + * a) seconary send request to primary.\n> + * b) primary attach the new device if failed goto i).\n> + * c) primary forward attach request to all secondary.\n> + * d) secondary receive request and attach device and send reply.\n> + * e) primary check the reply if all success go to j).\n> + * f) primary send attach rollback request to all secondary.\n> + * g) secondary receive the request and detach device and send reply.\n> + * h) primary receive the reply and detach device as rollback action.\n> + * i) send fail response to secondary, goto k).\n> + * j) send success response to secondary.\n> + * k) end.\n> +\n> + * device detach:\n> + * a) secondary send request to primary.\n> + * b) primary perform pre-detach check, if device is locked, got j).\n> + * c) primary send pre-detach check request to all secondary.\n> + * d) secondary perform pre-detach check and send reply.\n> + * e) primary check the reply if any fail goto j).\n> + * f) primary send detach request to all secondary\n> + * g) secondary detach the device and send reply\n> + * h) primary detach the device.\n> + * i) send success response to secondary, goto k).\n> + * j) send fail response to secondary.\n> + * k) end.\n> + */\n\nI think this comment should be at the top of this file.",
        "headers": {
            "To": "Qi Zhang <qi.z.zhang@intel.com>, thomas@monjalon.net",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "MIME-Version": "1.0",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "From": "\"Burakov, Anatoly\" <anatoly.burakov@intel.com>",
            "Message-ID": "<3146157e-4bc1-0662-45cb-0ae8d1626e6e@intel.com>",
            "Delivered-To": "patchwork@dpdk.org",
            "X-Amp-File-Uploaded": "False",
            "User-Agent": "Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.8.0",
            "In-Reply-To": "<20180607123849.14439-7-qi.z.zhang@intel.com>",
            "X-Amp-Result": "SKIPPED(no attachment in message)",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 2670A14E8;\n\tMon, 18 Jun 2018 10:51:14 +0200 (CEST)",
                "from mga05.intel.com (mga05.intel.com [192.55.52.43])\n\tby dpdk.org (Postfix) with ESMTP id CBB5F200\n\tfor <dev@dpdk.org>; Mon, 18 Jun 2018 10:51:12 +0200 (CEST)",
                "from fmsmga001.fm.intel.com ([10.253.24.23])\n\tby fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t18 Jun 2018 01:51:11 -0700",
                "from aburakov-mobl.ger.corp.intel.com (HELO [10.252.30.40])\n\t([10.252.30.40])\n\tby fmsmga001.fm.intel.com with ESMTP; 18 Jun 2018 01:51:09 -0700"
            ],
            "X-Original-To": "patchwork@dpdk.org",
            "Content-Type": "text/plain; charset=utf-8; format=flowed",
            "List-Post": "<mailto:dev@dpdk.org>",
            "Content-Transfer-Encoding": "7bit",
            "X-BeenThere": "dev@dpdk.org",
            "X-ExtLoop1": "1",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "Errors-To": "dev-bounces@dpdk.org",
            "X-IronPort-AV": "E=Sophos;i=\"5.51,238,1526367600\"; d=\"scan'208\";a=\"65011502\"",
            "References": "<20180607123849.14439-1-qi.z.zhang@intel.com>\n\t<20180607123849.14439-7-qi.z.zhang@intel.com>",
            "Subject": "Re: [dpdk-dev] [PATCH 06/22] ethdev: support attach or detach share\n\tdevice from secondary",
            "Date": "Mon, 18 Jun 2018 09:51:09 +0100",
            "Precedence": "list",
            "Content-Language": "en-US",
            "X-Mailman-Version": "2.1.15",
            "Cc": "konstantin.ananyev@intel.com, dev@dpdk.org, bruce.richardson@intel.com, \n\tferruh.yigit@intel.com, benjamin.h.shelton@intel.com,\n\tnarender.vangati@intel.com"
        }
    },
    {
        "id": 82362,
        "web_url": "http://patches.dpdk.org/comment/82362/",
        "msgid": "<039ED4275CED7440929022BC67E7061153239FC8@SHSMSX103.ccr.corp.intel.com>",
        "list_archive_url": "https://inbox.dpdk.org/dev/039ED4275CED7440929022BC67E7061153239FC8@SHSMSX103.ccr.corp.intel.com",
        "date": "2018-06-19T03:33:32",
        "subject": "Re: [dpdk-dev] [PATCH 06/22] ethdev: support attach or detach share\n\tdevice from secondary",
        "submitter": {
            "id": 504,
            "url": "http://patches.dpdk.org/api/people/504/",
            "name": "Qi Zhang",
            "email": "qi.z.zhang@intel.com"
        },
        "content": "> -----Original Message-----\n> From: Burakov, Anatoly\n> Sent: Monday, June 18, 2018 4:51 PM\n> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net\n> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;\n> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh\n> <ferruh.yigit@intel.com>; Shelton, Benjamin H\n> <benjamin.h.shelton@intel.com>; Vangati, Narender\n> <narender.vangati@intel.com>\n> Subject: Re: [PATCH 06/22] ethdev: support attach or detach share device\n> from secondary\n> \n> \n> > +\telse\n> > +\t\treturn -1;\n> > +\tdo {\n> > +\t\tret = rte_mp_request_async(&mp_req, &ts, clb);\n> > +\t} while (ret != 0 && rte_errno == EEXIST);\n> > +\n> > +\tif (ret)\n> > +\t\tethdev_log(ERR, \"couldn't send async request\\n\");\n> > +\tentry = find_request_by_id(req->id > +\t(void)entry;\n> \n> Why did you look up entry and then marked it as used without checking the\n> return value? Leftover? Some code missing?\n\nSome debug code forgot be removed :)\n\nBTW, also accept all other comments\n\nThanks\nQi",
        "headers": {
            "To": "\"Burakov, Anatoly\" <anatoly.burakov@intel.com>, \"thomas@monjalon.net\"\n\t<thomas@monjalon.net>",
            "x-originating-ip": "[10.239.127.40]",
            "x-titus-metadata-40": "eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjI0NTJkYTMtZmE0MS00ZjNlLWJlZTItY2U3MGNiYTE1ZWQ1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiaEVNdU1yMlpuQk9vQW9cL0t4ektCTEF3U1Y3bUNxQnZ6YVpXaUFVbHhtTk5FbDAzXC8wRlFCTUorT0Q2Z0E4UXNNIn0=",
            "x-ctpclassification": "CTP_NT",
            "Return-Path": "<dev-bounces@dpdk.org>",
            "MIME-Version": "1.0",
            "X-MS-TNEF-Correlator": "",
            "From": "\"Zhang, Qi Z\" <qi.z.zhang@intel.com>",
            "Message-ID": "<039ED4275CED7440929022BC67E7061153239FC8@SHSMSX103.ccr.corp.intel.com>",
            "CC": "\"Ananyev, Konstantin\" <konstantin.ananyev@intel.com>, \"dev@dpdk.org\"\n\t<dev@dpdk.org>, \"Richardson, Bruce\" <bruce.richardson@intel.com>,\n\t\"Yigit, Ferruh\" <ferruh.yigit@intel.com>, \"Shelton, Benjamin H\"\n\t<benjamin.h.shelton@intel.com>, \"Vangati, Narender\"\n\t<narender.vangati@intel.com>",
            "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Accept-Language": "en-US",
            "X-MS-Has-Attach": "",
            "References": "<20180607123849.14439-1-qi.z.zhang@intel.com>\n\t<20180607123849.14439-7-qi.z.zhang@intel.com>\n\t<3146157e-4bc1-0662-45cb-0ae8d1626e6e@intel.com>",
            "In-Reply-To": "<3146157e-4bc1-0662-45cb-0ae8d1626e6e@intel.com>",
            "Delivered-To": "patchwork@dpdk.org",
            "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
            "dlp-version": "11.0.200.100",
            "X-BeenThere": "dev@dpdk.org",
            "dlp-reaction": "no-action",
            "Received": [
                "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 5786019F5;\n\tTue, 19 Jun 2018 05:33:54 +0200 (CEST)",
                "from mga02.intel.com (mga02.intel.com [134.134.136.20])\n\tby dpdk.org (Postfix) with ESMTP id 9929814EC\n\tfor <dev@dpdk.org>; Tue, 19 Jun 2018 05:33:51 +0200 (CEST)",
                "from orsmga002.jf.intel.com ([10.7.209.21])\n\tby orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t18 Jun 2018 20:33:50 -0700",
                "from fmsmsx108.amr.corp.intel.com ([10.18.124.206])\n\tby orsmga002.jf.intel.com with ESMTP; 18 Jun 2018 20:33:36 -0700",
                "from fmsmsx125.amr.corp.intel.com (10.18.125.40) by\n\tFMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP\n\tServer (TLS) id 14.3.319.2; Mon, 18 Jun 2018 20:33:36 -0700",
                "from shsmsx102.ccr.corp.intel.com (10.239.4.154) by\n\tFMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP\n\tServer (TLS) id 14.3.319.2; Mon, 18 Jun 2018 20:33:36 -0700",
                "from shsmsx103.ccr.corp.intel.com ([169.254.4.51]) by\n\tshsmsx102.ccr.corp.intel.com ([169.254.2.223]) with mapi id\n\t14.03.0319.002; Tue, 19 Jun 2018 11:33:33 +0800"
            ],
            "X-Original-To": "patchwork@dpdk.org",
            "Content-Type": "text/plain; charset=\"utf-8\"",
            "List-Post": "<mailto:dev@dpdk.org>",
            "X-Amp-File-Uploaded": "False",
            "Content-Transfer-Encoding": "base64",
            "Thread-Index": "AQHUBuGEymyHO3Ygc0q7X7xfl0Wi16Rm7hCA",
            "X-IronPort-AV": "E=Sophos;i=\"5.51,241,1526367600\"; d=\"scan'208\";a=\"68082120\"",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "Errors-To": "dev-bounces@dpdk.org",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "X-ExtLoop1": "1",
            "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
            "Date": "Tue, 19 Jun 2018 03:33:32 +0000",
            "Thread-Topic": "[PATCH 06/22] ethdev: support attach or detach share device\n\tfrom secondary",
            "dlp-product": "dlpe-windows",
            "Subject": "Re: [dpdk-dev] [PATCH 06/22] ethdev: support attach or detach share\n\tdevice from secondary",
            "Precedence": "list",
            "Content-Language": "en-US",
            "X-Mailman-Version": "2.1.15",
            "X-Amp-Result": "SKIPPED(no attachment in message)"
        }
    }
]