Patch Comment List
List patch comments
GET /api/patches/185/comments/?format=api&order=-submitter
[ { "id": 775, "web_url": "https://patches.dpdk.org/comment/775/", "msgid": "<F35DEAC7BCE34641BA9FAC6BCA4A12E70A79272A@SHSMSX104.ccr.corp.intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/F35DEAC7BCE34641BA9FAC6BCA4A12E70A79272A@SHSMSX104.ccr.corp.intel.com", "date": "2014-09-15T00:21:45", "subject": "Re: [dpdk-dev] [PATCH 1/3] i40evf: support\n\tI40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host", "submitter": { "id": 14, "url": "https://patches.dpdk.org/api/people/14/?format=api", "name": "Zhang, Helin", "email": "helin.zhang@intel.com" }, "content": "Hi Thomas\n\n> -----Original Message-----\n> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]\n> Sent: Saturday, August 30, 2014 10:43 PM\n> To: Zhang, Helin\n> Cc: dev@dpdk.org\n> Subject: Re: [dpdk-dev] [PATCH 1/3] i40evf: support\n> I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host\n> \n> Hi Helin,\n> \n> The title mention i40evf but the patch is related to PF (for VF communication).\n> So I wonder wether it would be clearer to prefix it with i40e? Not sure.\n> \n> 2014-08-20 11:33, Helin Zhang:\n> > To configure VSI queues for VF, Linux PF host supports operation of\n> > I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES with limited configurations. To\n> > support more configurations (e.g configurable CRC stripping in VF), a\n> > new operation of I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX has been\n> > supported in DPDK PF host.\n> \n> This patch would be easier to read if you could split it in 3 patches:\n> 1) renamings and line wrapping rework\n> 2) introduction of new extended message/operation\n> 3) crc config\n> \n> > -\tint ret = I40E_SUCCESS;\n> > -\tstruct i40e_virtchnl_vsi_queue_config_info *qconfig =\n> > -\t (struct i40e_virtchnl_vsi_queue_config_info *)msg;\n> > -\tint i;\n> > -\tstruct i40e_virtchnl_queue_pair_info *qpair;\n> > -\n> > -\tif (msg == NULL || msglen <= sizeof(*qconfig) ||\n> > -\t\tqconfig->num_queue_pairs > vsi->nb_qps) {\n> > +\tstruct i40e_virtchnl_vsi_queue_config_info *vc_vqci =\n> > +\t\t(struct i40e_virtchnl_vsi_queue_config_info *)msg;\n> > +\tstruct i40e_virtchnl_queue_pair_info *vc_qpi;\n> > +\tstruct i40e_virtchnl_queue_pair_extra_info *vc_qpei = NULL;\n> > +\tint i, ret = I40E_SUCCESS;\n> > +\n> > +\tif (msg == NULL || msglen <= sizeof(*vc_vqci) ||\n> > +\t\tvc_vqci->num_queue_pairs > vsi->nb_qps) {\n> > \t\tPMD_DRV_LOG(ERR, \"vsi_queue_config_info argument wrong\\n\");\n> > \t\tret = I40E_ERR_PARAM;\n> > \t\tgoto send_msg;\n> > \t}\n> >\n> > -\tqpair = qconfig->qpair;\n> > -\tfor (i = 0; i < qconfig->num_queue_pairs; i++) {\n> > -\t\tif (qpair[i].rxq.queue_id > vsi->nb_qps - 1 ||\n> > -\t\t\tqpair[i].txq.queue_id > vsi->nb_qps - 1) {\n> > +\tvc_qpi = vc_vqci->qpair;\n> > +\tif (opcode == I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX)\n> > +\t\tvc_qpei = (struct i40e_virtchnl_queue_pair_extra_info *)\n> > +\t\t\t\t\t\t(((uint8_t *)vc_qpi) +\n> > +\t\t\t(sizeof(struct i40e_virtchnl_queue_pair_info) *\n> > +\t\t\t\t\tvc_vqci->num_queue_pairs));\n> > +\n> > +\tfor (i = 0; i < vc_vqci->num_queue_pairs; i++) {\n> > +\t\tif (vc_qpi[i].rxq.queue_id > vsi->nb_qps - 1 ||\n> > +\t\t\tvc_qpi[i].txq.queue_id > vsi->nb_qps - 1) {\n> > \t\t\tret = I40E_ERR_PARAM;\n> > \t\t\tgoto send_msg;\n> > \t\t}\n> \n> Mixing renaming and new feature makes it difficult to read.\n> \n> > \tcase I40E_VIRTCHNL_OP_ENABLE_QUEUES:\n> > \t\tPMD_DRV_LOG(INFO, \"OP_ENABLE_QUEUES received\\n\");\n> > -\t\ti40e_pf_host_process_cmd_enable_queues(vf,\n> > -\t\t\t\t\t\tmsg, msglen);\n> > +\t\ti40e_pf_host_process_cmd_enable_queues(vf, msg, msglen);\n> > \t\tbreak;\n> > \tcase I40E_VIRTCHNL_OP_DISABLE_QUEUES:\n> > \t\tPMD_DRV_LOG(INFO, \"OP_DISABLE_QUEUE received\\n\");\n> > -\t\ti40e_pf_host_process_cmd_disable_queues(vf,\n> > -\t\t\t\t\t\tmsg, msglen);\n> > +\t\ti40e_pf_host_process_cmd_disable_queues(vf, msg, msglen);\n> > \t\tbreak;\n> > \tcase I40E_VIRTCHNL_OP_ADD_ETHER_ADDRESS:\n> > \t\tPMD_DRV_LOG(INFO, \"OP_ADD_ETHER_ADDRESS received\\n\");\n> > -\t\ti40e_pf_host_process_cmd_add_ether_address(vf,\n> > -\t\t\t\t\t\tmsg, msglen);\n> > +\t\ti40e_pf_host_process_cmd_add_ether_address(vf, msg, msglen);\n> > \t\tbreak;\n> > \tcase I40E_VIRTCHNL_OP_DEL_ETHER_ADDRESS:\n> > \t\tPMD_DRV_LOG(INFO, \"OP_DEL_ETHER_ADDRESS received\\n\");\n> > -\t\ti40e_pf_host_process_cmd_del_ether_address(vf,\n> > -\t\t\t\t\t\tmsg, msglen);\n> > +\t\ti40e_pf_host_process_cmd_del_ether_address(vf, msg, msglen);\n> > \t\tbreak;\n> > \tcase I40E_VIRTCHNL_OP_ADD_VLAN:\n> > \t\tPMD_DRV_LOG(INFO, \"OP_ADD_VLAN received\\n\"); @@ -932,10\n> +951,9 @@\n> > i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,\n> > \tcase I40E_VIRTCHNL_OP_FCOE:\n> > \t\tPMD_DRV_LOG(ERR, \"OP_FCOE received, not supported\\n\");\n> > \tdefault:\n> > -\t\tPMD_DRV_LOG(ERR, \"%u received, not supported\\n\",\n> > -\t\t\t\t\t\t\topcode);\n> > -\t\ti40e_pf_host_send_msg_to_vf(vf, opcode,\n> > -\t\t\t\tI40E_ERR_PARAM, NULL, 0);\n> > +\t\tPMD_DRV_LOG(ERR, \"%u received, not supported\\n\", opcode);\n> > +\t\ti40e_pf_host_send_msg_to_vf(vf, opcode, I40E_ERR_PARAM,\n> > +\t\t\t\t\t\t\t\tNULL, 0);\n> > \t\tbreak;\n> > \t}\n> \n> Line wrapping should go in a cleanup patch (like renaming).\n> \n> Thanks\n> --\n> Thomas\n\nThank you very much! I have reworked the patches and sent out again.\n\nRegards,\nHelin", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id C57F03976;\n\tMon, 15 Sep 2014 02:16:22 +0200 (CEST)", "from mga01.intel.com (mga01.intel.com [192.55.52.88])\n\tby dpdk.org (Postfix) with ESMTP id 372C92E81\n\tfor <dev@dpdk.org>; Mon, 15 Sep 2014 02:16:20 +0200 (CEST)", "from fmsmga002.fm.intel.com ([10.253.24.26])\n\tby fmsmga101.fm.intel.com with ESMTP; 14 Sep 2014 17:21:48 -0700", "from fmsmsx107.amr.corp.intel.com ([10.18.124.205])\n\tby fmsmga002.fm.intel.com with ESMTP; 14 Sep 2014 17:21:47 -0700", "from fmsmsx115.amr.corp.intel.com (10.18.116.19) by\n\tfmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP\n\tServer (TLS) id 14.3.195.1; Sun, 14 Sep 2014 17:21:47 -0700", "from shsmsx103.ccr.corp.intel.com (10.239.4.69) by\n\tfmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP\n\tServer (TLS) id 14.3.195.1; Sun, 14 Sep 2014 17:21:47 -0700", "from shsmsx104.ccr.corp.intel.com ([169.254.5.230]) by\n\tSHSMSX103.ccr.corp.intel.com ([169.254.4.204]) with mapi id\n\t14.03.0195.001; Mon, 15 Sep 2014 08:21:45 +0800" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.04,523,1406617200\"; d=\"scan'208\";a=\"599472331\"", "From": "\"Zhang, Helin\" <helin.zhang@intel.com>", "To": "Thomas Monjalon <thomas.monjalon@6wind.com>", "Thread-Topic": "[dpdk-dev] [PATCH 1/3] i40evf: support\n\tI40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host", "Thread-Index": "AQHPxGEpdMCLhjzuJkGGgIrdmZE0RZwBbRpA", "Date": "Mon, 15 Sep 2014 00:21:45 +0000", "Message-ID": "<F35DEAC7BCE34641BA9FAC6BCA4A12E70A79272A@SHSMSX104.ccr.corp.intel.com>", "References": "<1408505611-6959-1-git-send-email-helin.zhang@intel.com>\n\t<1408505611-6959-2-git-send-email-helin.zhang@intel.com>\n\t<1897473.c67KeMpIux@xps13>", "In-Reply-To": "<1897473.c67KeMpIux@xps13>", "Accept-Language": "en-US", "Content-Language": "en-US", "X-MS-Has-Attach": "", "X-MS-TNEF-Correlator": "", "x-originating-ip": "[10.239.127.40]", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "quoted-printable", "MIME-Version": "1.0", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH 1/3] i40evf: support\n\tI40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null }, { "id": 581, "web_url": "https://patches.dpdk.org/comment/581/", "msgid": "<1897473.c67KeMpIux@xps13>", "list_archive_url": "https://inbox.dpdk.org/dev/1897473.c67KeMpIux@xps13", "date": "2014-08-30T14:43:15", "subject": "Re: [dpdk-dev] [PATCH 1/3] i40evf: support\n\tI40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host", "submitter": { "id": 1, "url": "https://patches.dpdk.org/api/people/1/?format=api", "name": "Thomas Monjalon", "email": "thomas.monjalon@6wind.com" }, "content": "Hi Helin,\n\nThe title mention i40evf but the patch is related to PF (for VF communication).\nSo I wonder wether it would be clearer to prefix it with i40e? Not sure.\n\n2014-08-20 11:33, Helin Zhang:\n> To configure VSI queues for VF, Linux PF host supports\n> operation of I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES with\n> limited configurations. To support more configurations\n> (e.g configurable CRC stripping in VF), a new operation\n> of I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX has been\n> supported in DPDK PF host.\n\nThis patch would be easier to read if you could split it in 3 patches:\n1) renamings and line wrapping rework\n2) introduction of new extended message/operation\n3) crc config\n\n> -\tint ret = I40E_SUCCESS;\n> -\tstruct i40e_virtchnl_vsi_queue_config_info *qconfig =\n> -\t (struct i40e_virtchnl_vsi_queue_config_info *)msg;\n> -\tint i;\n> -\tstruct i40e_virtchnl_queue_pair_info *qpair;\n> -\n> -\tif (msg == NULL || msglen <= sizeof(*qconfig) ||\n> -\t\tqconfig->num_queue_pairs > vsi->nb_qps) {\n> +\tstruct i40e_virtchnl_vsi_queue_config_info *vc_vqci =\n> +\t\t(struct i40e_virtchnl_vsi_queue_config_info *)msg;\n> +\tstruct i40e_virtchnl_queue_pair_info *vc_qpi;\n> +\tstruct i40e_virtchnl_queue_pair_extra_info *vc_qpei = NULL;\n> +\tint i, ret = I40E_SUCCESS;\n> +\n> +\tif (msg == NULL || msglen <= sizeof(*vc_vqci) ||\n> +\t\tvc_vqci->num_queue_pairs > vsi->nb_qps) {\n> \t\tPMD_DRV_LOG(ERR, \"vsi_queue_config_info argument wrong\\n\");\n> \t\tret = I40E_ERR_PARAM;\n> \t\tgoto send_msg;\n> \t}\n> \n> -\tqpair = qconfig->qpair;\n> -\tfor (i = 0; i < qconfig->num_queue_pairs; i++) {\n> -\t\tif (qpair[i].rxq.queue_id > vsi->nb_qps - 1 ||\n> -\t\t\tqpair[i].txq.queue_id > vsi->nb_qps - 1) {\n> +\tvc_qpi = vc_vqci->qpair;\n> +\tif (opcode == I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX)\n> +\t\tvc_qpei = (struct i40e_virtchnl_queue_pair_extra_info *)\n> +\t\t\t\t\t\t(((uint8_t *)vc_qpi) +\n> +\t\t\t(sizeof(struct i40e_virtchnl_queue_pair_info) *\n> +\t\t\t\t\tvc_vqci->num_queue_pairs));\n> +\n> +\tfor (i = 0; i < vc_vqci->num_queue_pairs; i++) {\n> +\t\tif (vc_qpi[i].rxq.queue_id > vsi->nb_qps - 1 ||\n> +\t\t\tvc_qpi[i].txq.queue_id > vsi->nb_qps - 1) {\n> \t\t\tret = I40E_ERR_PARAM;\n> \t\t\tgoto send_msg;\n> \t\t}\n\nMixing renaming and new feature makes it difficult to read.\n\n> \tcase I40E_VIRTCHNL_OP_ENABLE_QUEUES:\n> \t\tPMD_DRV_LOG(INFO, \"OP_ENABLE_QUEUES received\\n\");\n> -\t\ti40e_pf_host_process_cmd_enable_queues(vf,\n> -\t\t\t\t\t\tmsg, msglen);\n> +\t\ti40e_pf_host_process_cmd_enable_queues(vf, msg, msglen);\n> \t\tbreak;\n> \tcase I40E_VIRTCHNL_OP_DISABLE_QUEUES:\n> \t\tPMD_DRV_LOG(INFO, \"OP_DISABLE_QUEUE received\\n\");\n> -\t\ti40e_pf_host_process_cmd_disable_queues(vf,\n> -\t\t\t\t\t\tmsg, msglen);\n> +\t\ti40e_pf_host_process_cmd_disable_queues(vf, msg, msglen);\n> \t\tbreak;\n> \tcase I40E_VIRTCHNL_OP_ADD_ETHER_ADDRESS:\n> \t\tPMD_DRV_LOG(INFO, \"OP_ADD_ETHER_ADDRESS received\\n\");\n> -\t\ti40e_pf_host_process_cmd_add_ether_address(vf,\n> -\t\t\t\t\t\tmsg, msglen);\n> +\t\ti40e_pf_host_process_cmd_add_ether_address(vf, msg, msglen);\n> \t\tbreak;\n> \tcase I40E_VIRTCHNL_OP_DEL_ETHER_ADDRESS:\n> \t\tPMD_DRV_LOG(INFO, \"OP_DEL_ETHER_ADDRESS received\\n\");\n> -\t\ti40e_pf_host_process_cmd_del_ether_address(vf,\n> -\t\t\t\t\t\tmsg, msglen);\n> +\t\ti40e_pf_host_process_cmd_del_ether_address(vf, msg, msglen);\n> \t\tbreak;\n> \tcase I40E_VIRTCHNL_OP_ADD_VLAN:\n> \t\tPMD_DRV_LOG(INFO, \"OP_ADD_VLAN received\\n\");\n> @@ -932,10 +951,9 @@ i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,\n> \tcase I40E_VIRTCHNL_OP_FCOE:\n> \t\tPMD_DRV_LOG(ERR, \"OP_FCOE received, not supported\\n\");\n> \tdefault:\n> -\t\tPMD_DRV_LOG(ERR, \"%u received, not supported\\n\",\n> -\t\t\t\t\t\t\topcode);\n> -\t\ti40e_pf_host_send_msg_to_vf(vf, opcode,\n> -\t\t\t\tI40E_ERR_PARAM, NULL, 0);\n> +\t\tPMD_DRV_LOG(ERR, \"%u received, not supported\\n\", opcode);\n> +\t\ti40e_pf_host_send_msg_to_vf(vf, opcode, I40E_ERR_PARAM,\n> +\t\t\t\t\t\t\t\tNULL, 0);\n> \t\tbreak;\n> \t}\n\nLine wrapping should go in a cleanup patch (like renaming).\n\nThanks", "headers": { "Return-Path": "<thomas.monjalon@6wind.com>", "Received": [ "from mail-we0-f181.google.com (mail-we0-f181.google.com\n\t[74.125.82.181]) by dpdk.org (Postfix) with ESMTP id 96DCCB378\n\tfor <dev@dpdk.org>; Sat, 30 Aug 2014 16:41:50 +0200 (CEST)", "by mail-we0-f181.google.com with SMTP id x48so3450589wes.40\n\tfor <dev@dpdk.org>; Sat, 30 Aug 2014 07:46:09 -0700 (PDT)", "from xps13.localnet (APoitiers-651-1-232-7.w2-6.abo.wanadoo.fr.\n\t[2.6.255.7]) by mx.google.com with ESMTPSA id\n\tdh7sm6091373wib.18.2014.08.30.07.46.07 for <multiple recipients>\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tSat, 30 Aug 2014 07:46:08 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:organization\n\t:user-agent:in-reply-to:references:mime-version\n\t:content-transfer-encoding:content-type;\n\tbh=IVhu8FGiZSQwRw+HBe2fJ2FL5SyLL78jETQJ+TJCG/k=;\n\tb=l9wRf/K3T4IBLjtjy9W1z32cMD8gIiJYfxUgTKMiSgrYoEPne8HxGU5IL9OmGu12rH\n\tY3qZWgdprGvxuV5CjwX5T4ZGecuq259V4zJyUn1ayjvQlJoPXhknvvRMNI1KPYLfrrtS\n\t2GnWcaf487YwDYdQWfbe+rexDzjFNUMJbSCBVgfNc8wu5gdhlA0Y1GUQM92ZWt9ghzIP\n\tapFmOL50f0kmevKjpDr5Y26NaVLRWdGYjkbgAvJjXDp1K2/ZGFuvCCMoHa/PkHKJ2KRa\n\tfNvvP8myl+sgWz4cH8qYoTXik8jVUAmIPYfSBv0f7JKK+RrIE8l6r+Z+YKHuT4IHBONs\n\tHv+g==", "X-Gm-Message-State": "ALoCoQn9RSQUha93CEYKQZvM+f84NfjFOl5g5gY6kFclVRVxDF3wzzGlT3sjTMZIc6zwH9UJZcX3", "X-Received": "by 10.180.149.197 with SMTP id uc5mr10378410wib.75.1409409969085;\n\tSat, 30 Aug 2014 07:46:09 -0700 (PDT)", "From": "Thomas Monjalon <thomas.monjalon@6wind.com>", "To": "Helin Zhang <helin.zhang@intel.com>", "Date": "Sat, 30 Aug 2014 16:43:15 +0200", "Message-ID": "<1897473.c67KeMpIux@xps13>", "Organization": "6WIND", "User-Agent": "KMail/4.13.3 (Linux/3.15.8-1-ARCH; KDE/4.13.3; x86_64; ; )", "In-Reply-To": "<1408505611-6959-2-git-send-email-helin.zhang@intel.com>", "References": "<1408505611-6959-1-git-send-email-helin.zhang@intel.com>\n\t<1408505611-6959-2-git-send-email-helin.zhang@intel.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "7Bit", "Content-Type": "text/plain; charset=\"us-ascii\"", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH 1/3] i40evf: support\n\tI40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "X-List-Received-Date": "Sat, 30 Aug 2014 14:41:50 -0000" }, "addressed": null } ]